14

In our code base we have many constructions like this:

auto* pObj = getObjectThatMayVeryRarelyBeNull();
if (!pObj) throw std::runtime_error("Ooops!");
// Use pObj->(...)

In 99.99% of cases this check is not triggered. I'm thinking about the following solution:

auto& obj = deref_or_throw(getObjectThatMayVeryRarelyBeNull());
// Use obj.(...)

Where deref_or_throw is declared as follows:

template<class T> T& deref_or_throw(T* p) {
  if (p == nullptr) { throw std::invalid_argument("Argument is null!"); }
  return *p;
}

That code is much clearer and works as I need.

The question is: am I reinventing the wheel? Is there some related solution in standard or boost? Or do you have some comments on the solution?

PS. Related question (with no satisfying answer): Is there a C++ equivalent of a NullPointerException

Community
  • 1
  • 1
Mikhail
  • 20,685
  • 7
  • 70
  • 146
  • Is there any code at all that actually deals with the `nullptr` case? E.g. anything like `if ( !obj ) { std::cout << "Null, but still okay, moving on with something meaningful\n"; } else { std::cout << "Non-Null, doing something else\n"; }`. Or is the `nullptr` case always an "exit, because this is wrong"-path? – stefan Mar 18 '15 at 12:41
  • This code exists, but is rare. In most of the cases we just cannot proceed. Nothing we can do. The whole routine is meaningless. So maybe existence of this object is a _precondition_? And I need to check it and then manually dereference the pointer without worrying? – Mikhail Mar 18 '15 at 12:44
  • 1
    Exactly this is what you need to clarify for yourself, imho. If it's a precondition, maybe an `assert`ion is a better fit. Otherwise, this is clearly an exceptional thing (0.01% is fairly exceptional), so an `exception` is appropriate. Afaik, there is no such thing in a library. – stefan Mar 18 '15 at 12:52
  • I think you should elaborate on this topic and post it as an answer. – Mikhail Mar 18 '15 at 13:07
  • What if you throw an exception from getObjectThatMayVeryRarelyBeNull function ? – zapredelom Mar 18 '15 at 13:24
  • @zapredelom I cannot modify this function. – Mikhail Mar 18 '15 at 14:12
  • And... what about the end of that object's lifespan? Does the pointer need deletion (or some other disposal method)? If so, then turning that into a reference is hiding the lifespan issue (possibly leading to leaks). – Andre Kostur Mar 18 '15 at 15:48
  • Regarding the pointer, perhaps leaving it as `auto *`? I would have suggested `auto`, but wanted to keep the highlight that this is a pointer. Or perhaps better, a `std::unique_ptr` with an appropriate deleter? But if the caller isn't responsible for any lifespan decisions, then an `auto &` would probably be best. – Andre Kostur Mar 18 '15 at 15:55
  • @AndreKostur Object lifespan is irrelevant in this question. The object is guaranteed to outlive any considered routine. – Mikhail Mar 18 '15 at 18:30

2 Answers2

4

There are two ways of dealing with the "rare case of null-pointer" issue. First, it's the proposed exception solution. For this, the method deref_or_throw is a good thing, though I would prefer to throw a runtime_error, rather than an invalid_argument exception. You may also consider naming it NullPointerException, if you prefer that.

However, if the ptr != nullptr case is actually a precondition to the algorithm, you should try your best to achieve 100% safety in having this non-null case. An assert is the appropriate thing in this case.

Advantages of assert:

  • No cost at runtime in release mode
  • Makes it crystal clear to the developer, that he is responsible for this to never happen.

Disadvantages of assert:

  • Potential undefined behavior in release mode

You should also consider writing a method getObjectThatMayNeverBeNullButDoingTheSameAsGetObjectThatMayVeryRarelyBeNull(): A method that is guaranteed to return a non-null pointer to an object that is otherwise completely equivalent to your original method. However, if you can go the extra mile, I would strongly suggest to not returning a raw pointer anyway. Embrace C++11 and return objects by value :-)

stefan
  • 10,215
  • 4
  • 49
  • 90
  • I would like to have assertions, which are not deleted in release. And I would like to distinguish between assertions and preconditions. Any good links on that topic? – Mikhail Mar 18 '15 at 18:33
  • Assertions are preconditions (or at least the closest thing to it). But nothing stops you from using both the assertion and the exception. :) – stefan Mar 18 '15 at 18:35
  • Yeah, solutions are so obvious, that I'm sure someone did a good job on it and has a consistent and general framework on it. I don't know of any, though. Boost.assert has no preconditions, for example. – Mikhail Mar 19 '15 at 11:21
1

You can, and probably should, design with null cases in mind. For example, in your problem domain, when does it make sense for class methods to return null? When does it make sense to call functions with null arguments?

Broadly speaking, it can be beneficial to remove null references from code whenever possible. In other words, you can program to the invariant that "this will not be null here... or there". This has many benefits. You won't have to obfuscate code by wrapping methods in deref_or_throw. You may achieve more semantic meaning from code, because, how often are things null in the real world? Your code may be more readable if you use exceptions to indicate errors rather than null values. And finally, you reduce the need for error-checking and also reduce the risk of run time errors (the dreaded null pointer dereference).

If your system was not designed with null cases in mind, I'd say it's best to leave it alone and not go crazy wrapping everything with deref_or_throw. Perhaps take an Agile approach. As you are coding, inspect the contracts that your class objects offer as services. How often can these contracts reasonably be expected to return null? What is the semantic value of null in these cases?

You could probably identify the classes in your system that might reasonably be expected to return null. For these classes, null may be valid business logic, rather than fringe cases that are indicative of implementation errors. For the classes that might be expected to return null, the additional check may be worth the safety. In this sense, checking for null feels more like business logic rather than low level implementation details. Across the board, though, systematically wrapping all pointer deference in deref_or_throw might be a cure that is its own poison.

  • Function `getObjectThatMayVeryRarelyBeNull()` should have a possibility to return `null`. It is rare, but it is possible. But since it is rare, most of routines (except some special ones) expect it to return an object. Otherwise they are simply meaningless. – Mikhail Mar 18 '15 at 13:06
  • I would be tempted to remove null as a valid return value, and perhaps consider another option to indicate that very rare case. Without knowing your use case, I can't speculate on exactly what that'd be. Seems like your class can return a "surprising" result that not all dependents would handle properly, so it seems dangerous indeed to return null there. And of course, future readers of your code shouldn't have to pick up on the subtlety that `getObjectThatMayVeryRarelyBeNull()` returns null 0.01% of the time –  Mar 18 '15 at 13:08
  • This is indeed a tricky design question, and that's why I'm not asking it :) I restrict the area of possible solutions so that interface of `getObjectThatMayVeryRarelyBeNull()` cannot be changed. – Mikhail Mar 18 '15 at 13:16
  • 1
    @Mikhail *interface of `getObjectThatMayVeryRarelyBeNull()` cannot be changed* Ok, but... is possible to check inside the function if it is going to return null and in that cases throw an exception from inside the function? – PaperBirdMaster Mar 18 '15 at 13:51
  • 1
    @PaperBirdMaster This is possible, but it is a breaking change. Because actually it _is_ an interface change (though binary compatible), because we remove implicit `noexcept`. – Mikhail Mar 18 '15 at 14:14
  • @mikhail You mention "most of routines expect it to return an object". If `nullptr` is a valid return, and those functions don't deal with it, then it is those functions that are broken. It doesn't matter if it's rare. It can happen thus you must deal with it. Particularly since the consequences of ignoring it are dereferencing null pointers and thus Undefined Behaviour. – Andre Kostur Mar 18 '15 at 15:52
  • @AndreKostur These functions are not broken, they are just applicable in the most common scenario only. – Mikhail Mar 18 '15 at 18:29