C++ Pointers – When to Assert for Non-Null Pointers

assertionscpointers

This came up as part of a code review for a code segment which resembled this:

auto somePikachu = GetMeAPikachu();
NT_ASSERT(somePikachu != nullptr); // this only fires on debug build
somePikachu->ThunderBolt();

In my opinion that assert is redundant since if somePikachu is null it will crash anyway in the next line when it tries to call Thunderbolt. (Of course if ThunderBolt doesn't require dereferencing this pointer nothing will happen on release build). So the assert is not buying anything IMHO.
There is another school of thought that was of the opinion that the assert adds value to readability and makes the intent of the design clear.
I would like to get the opinion of the community on the correct pattern of asserting the design for validity of pointers.
Please feel free to include other possible scenarios that match the problem statement.

Best Answer

The value of the assertion is:

  • It explicitly documents the assumption of the programmer that GetMeAPikachu returns non-null. This is especially valuable if the function at some point in history changed behavior, or if there are specific circumstances in which the function can return null, but the programmer is asserting that these circumstances don't apply here.
  • It turns the error situation from "there was a crash, now go and find the reason" into "this pointer was null".
  • It is independent of the implementation details of ThunderBolt. If that function isn't virtual and doesn't access member variables itself, the crash could simply not happen at all, or be deep within the call tree, and it might be not an access violation, but possibly an illegal instruction because you eventually managed to jump through an invalid function pointer/vtable.