How bad is "if (!this)" in a C++ member function?

Solution 1:

I would leave it alone. This might have been a deliberate choice as an old-fashioned version of the SafeNavigationOperator. As you say, in new code, I wouldn't recommend it, but for existing code, I'd leave it alone. If you do end up modifying it, I'd make sure that all calls to it are well-covered by tests.

Edit to add: you could choose to remove it only in debug versions of your code via:

CThingy *CLookupThingy::Lookup( name ) 
{
#if !defined(DEBUG)
   if (!this)
   {
      return NULL;
   }
#endif
   // else do the lookup code...
}

Thus, it wouldn't break anything on production code, while giving you a chance to test it in debug mode.

Solution 2:

Like all undefined behavior

if (!this)
{
   return NULL;
}

this is a bomb waiting to go off. If it works with your current compilers, you are kind-of lucky, kind-of unlucky!

The next release of the same compilers might be more aggressive and see this as dead code. As this can never be null, the code can "safely" be removed.

I think it is better if you removed it!