Coding style - call method on a pointer after NULL check inside if condition

Question

Say we have two pointers to objects that can contain a property equal and we want to check it. But first, we need to check whether the pointers are initialized or not. What option is usually preferred? will it have some micro impact on performance?

if(p1 && p2 && p1->getA() == p2->getB()){
    execute fancy code
}

or:

if(p1 && p2){
    if(p1->getA() == p2->getB()){
        execute fancy code
    }
}

I was wondering what is usual preferred.

Thanks in advance.

Answer 1

First, if (ptr) doesn't check whether a pointer is intialised or not, it checks if it isn't NULL. You can initialize a pointer to NULL and the condition wouldn't hold.

Second, there are two cases to be treated:

1) the pointers are allowed by the code logic to be NULL

In this case, you most certainly want different behavior for the two cases. So what would be appropriate is:

if ( ptr )
{  
   ptr->foo();
   //...
}
else
{
   //...
}

The second syntax doesn't make much sense semantically if ( ptr && ptr->foo() ) implies that you want to be sure ptr isn't NULL before calling foo(), instead of grouping the logic bound to the case where foo isn't false into a use-case.

2) the pointers aren't allowed to be NULL

If they're not allowed to be NULL, then you should deal with the case that they are NULL, not by excluding it completely, which is what

if ( ptr && ptr->foo() )

does. But by making it burn:

if ( !ptr )
    throw std::exception("WTF! This shouldn't be NULL");

ptr && ptr->foo() seems like it's meant to prevent crashes, but at the same time hide bugs. In a clean logic, you wouldn't need to check for NULL. If the object wasn't created and the pointer didn't point to anything meaningful, you'd have a bug regardless of whether you call foo or not, so you should deal with it, not hide it behind a check.