Should I always verify if an object is NULL ?

Question

Hello,

I have a object built through a factory containing my parameters read from the url.

From this object, I can get the language parameter

$language = $my_parameters->getLanguage();

$language is NULL if it isn't been set.

$language may also be invalid ( $language->isValid() returns false ).

So, to build my page, I need some parameters.

The page is also built trough a factory. Then I know which parameters I need to build it. If it miss parameters, I build them with a valid default value according to the page asked.

At this point, into the page factory, if a have an invalid parameter, I throw an exception.

My page object contains a body object that needs a language parameter. I know that my parameters are valid when I build my body object.

Into my body object, I retrieve the language

$language = $my_parameters->getLanguage();

At this point, $language ** MUST ** be valid. So I verify again

$language = $my_parameters->getLanguage();
if( is_null( $language ) or !$language->isValid() ) {
   throw new Exception( 'Language must be valid.' );
}

If I need 4 parameters, I have 4 ifs that verify if the object is not NULL and not invalid.

I do it because the method is public where $language is used in the body object .

And the body object may be built outside the factory. Who knows...

Is it correct to verify in that case ?

What are the best-practices about that ?

Answer 1

i know very little about your domain, but in the general case, i like to assert(not null) all over the place, because typically if i end up with a null object somewhere, its a bug.

it's also good practice to prefer reference types which typically can't even be null.

Answer 2

Here is the case for not checking for null in a recent blog post from the Google Testing blog.

The argument is that it gets in the way of writing clear, easy unit tests, because you can't actually fake the pieces that don't matter, because your exceptions/assertions will be thrown.

The author (Miško Hevery) does qualify the commentary by saying that if it's an external API, it might still be worth checking for an error condition.

Answer 3

I come from a very old school of C programming; so my thing is that variables that are not being used or that have been free()'d should always be NULL. That's just my opinion though.

Edit: Building on that, you should always check to see if a variable is NULL before using it as well. If the variable is NULL and shouldn't be then you should log an error. Crashing should not be a feature.

Answer 4

You can make life simpler for yourself by splitting the getLanguage() into two methods:

function getLanguageIfValid() {
  // this method return a Language object, but only if it can be created
  // correctly and the isValid() method returns TRUE. If the Language object
  // can't be created correctly, then it will return null instead.
}

function getLanguageMustBeValid() {
    // this method will always return an instance of Language, or else
    // an exception will be thrown
    if($return = $this->getLanguageIfValid())
        return $return;
    throw new Exception("Couldn't get Language object");
}

Once you've done that, in places where it is reasonable to say that the language item might not be created properly, you use the first method:

// we may or may not be able to get our Language object here
if($language = $my_parameters->getLanguageIfValid())
    do_stuff($language);

If you're certain the language object should be created, then use the 2nd method which will throw the exception for you.

// we know the Language object is created at this point
$language = $my_parameters->getLanguageMustBeValid();
do_stuff($language);

So the answer to your question is No - you don't have to verify an object is not null as long as you can get it from a function that us guaranteed not to return null.

Answer 5

Throw your exception from ->getLanguage().

To me, exceptions should be thrown automatically. What you are doing seems like a mixture of error-code checkup and exception throwing.