1

Assume I have the following class hierarchy:

A Person object contains a Customer object and the Customer object contains an Address object.

I do not care if the customer has an address or not, but if they do, I want to save it to the database. So in this case, is it perfectly fine to have something like:

try
{
     Address addr = person.Customer.Address;
     db.SaveAddress(addr);
}
catch
{
    //I don't care, but if it is there, just save it.
}

In the above, I don't care if Customer is null or Address is null. The other alternative I had was

 if(person.Customer != null)
 {
     if(person.Customer.Address != null)
     {

     }
 }

The above can long though if the hierarchy is long. Is there a more elegant way to check if a chain of objects is null without having to check each one.

Xaisoft
  • 45,655
  • 87
  • 279
  • 432
  • take a look at this: [how-to-avoid-null-statements-in-java](http://stackoverflow.com/questions/271526/how-to-avoid-null-statements-in-java) – Kerem Baydoğan Jun 27 '11 at 20:27

5 Answers5

5

You should always use exceptions for exceptional cases. You will suffer through performance penalties as there will be some context switching on the CPU.

You could combine the 2nd example to 1 line with most short circuiting languages.

 if(person.Customer != null && person.Customer.Address != null)
 {

 }

Example:

 bool isAddressValid = person.Customer != null && person.Customer.Address != null;
 if (isAddressIsValid) { }

Context Switching: http://en.wikipedia.org/wiki/Context_switch

Daniel A. White
  • 187,200
  • 47
  • 362
  • 445
3

There are several reasons to have your code check the condition, instead of just catching any exception and ignoring it. The first two that come to mind are:

  1. Like other people mentioned, using exceptions has a very high performance cost (when exception are actually thrown). For a valid and common scenario, you would usually prefer to get an indication of an error through some condition or method return value.

  2. Even if you do detect a possible condition by catching an exception, it is recommended to throw a specific exception type, and only catch the type of exception you allow. Because what if something else went wrong and a different exception was thrown? (for example what if Save through an OutOfMemoryException? You would have caught and ignored it). In your case, even if you went for catching and ignoring an exception, it would be better to only catch NullReferenceException, and ignore the rest. Even better would have been define your own exception type (e.g. CustomerHasNoAddressException) and only catch that.

Ran
  • 5,989
  • 1
  • 24
  • 26
2

You should always check for null (or whatever condition) if you know it may occur and you can avoid the exception, the reason for this being that exceptions are slow, and as you point out, inelegant. Just use:

if(person.Customer != null && person.Customer.Address != null) {
    // ...
}
Ry-
  • 218,210
  • 55
  • 464
  • 476
2

That is a very bad use of an Exception. There are sometimes "questionable" and "sneaky" places, but that is just downright ... bad.

It is bad because you are covering up every possible exception inside there, and adding no self-documentation to the code as to what may be covered up.

What if SavePerson could not save the person? :( Everything would happily continue (well, except for Mr. Fred who isn't saved... he might be upset.)

The code with the explicit "null-check" isn't plagued with this issue, and it adds self-documentation. It is evident that a null case is anticipated (whether that reasoning is valid or not is another story). If the number of indentation levels is the issue, consider internal methods with a contract of not accepting null values.

Happy coding.

  • @pst, I did not write this. I don't like the idea. I am trying to reason as to why someone would have a bunch of empty try/catch statements. – Xaisoft Jun 27 '11 at 20:32
  • @Xaisoft I don't try to reason bad code :) Most people write *absolutely awful* code. –  Jun 27 '11 at 20:33
  • @pst, Also, the example wasn't about Saving a Person, it was about saving the address of a person and what if you do not care if the address is saved or not, so if Mr. Fred's address isn't saved, you don't care, just move on to Barney :) – Xaisoft Jun 27 '11 at 20:35
  • @Xaisoft The point stands, I was trying to make it a bit of hyperbole -- something was *supposed* to happen, but *might not have*. –  Jun 27 '11 at 20:36
  • Can you elaborate on what you mean by "consider internal methods with a contract of not accepting null values." – Xaisoft Jun 28 '11 at 13:42
  • @Xaisoft `/* Get the persons real name. Person can't be null. */ private void GetRealName (Person person) { assert(x != null); ... }` -- Because I have given a contract that `person` can't be null, any code that calls this is with a `null` value violates the contract and is therefore a bug. Note that because this is an *internal* method it is not part of the ABI/API. The API layer might be: `void GetStats (Person p) { if (p != null) { realName = GetRealName(p); /* can't be null here */ } ... }` Well, that was a silly/lame example :-) In any case, coding is to *contracts*, implicit or explicit. –  Jun 28 '11 at 17:01
1

It is an extremely bad practice to use exceptions for ordinary control flow. The try-catch variant can be up to 1000 times slower than with the null checks due to stack unwinding.

Bohdan
  • 1,987
  • 16
  • 23