3

Consider the following piece of code

class Foo
    {
    public:
    //...
        bool valueFirstGet(int& value) const
            {
            if(this==nullptr)
                {return 0;}
            value=values[0];
            return 1;
            }
    //...

    private:
        int* values;
        size_t n_values;
    };

int main()
    {
    Foo* obj=findObject("key");
    int value;
    if(!obj->valueFirstGet(value))
        {printf("key does not exist\n");}

    return 0;
    }

findObject returns nullptr if it cannot find the object. Is it ok to let the member function do the null check instead of its caller. In my code, there are several calls to findObject directly followed by a call to valueFirstGet so leaving the check to the caller makes the code ugly.

EDIT:

Is there a cleaner way to avoid all null checking besides having findObject to throw an exception instead of returning null?

EDIT 2:

What about a static wrapper?

7
  • 5
    calling valueFirstGet on a null pointer is undefined behavior Commented Jan 7, 2014 at 10:07
  • 5
    Remember that "works as expected" is a valid result from undefined behavior (as is "brick the machine") Commented Jan 7, 2014 at 10:37
  • 1
    The sane solution is to throw an exception on failure. Nulls are one of the biggest mistakes modern languages continue to perpetuate, and they're only just starting to realize this. If you can't use exceptions for performance or standards reasons, consider either a hasObject(String): bool method or a tryFindObject(String, out Foo*): bool. Null pointers are an error condition, not a valid return result.
    – Phoshi
    Commented Jan 7, 2014 at 10:38
  • 1
    @Phoshi In my situation, a missing key does not necessary imply a failure. Therefore, findObject should not throw.
    – user877329
    Commented Jan 7, 2014 at 10:41
  • 2
    @user877329: Then ignore that bit and take the rest of the post. Null pointers are still an error condition, and without a Maybe or Option type I think your best bet is something like tryFindObject or hasObject. Nulls are harmful, avoid if possible.
    – Phoshi
    Commented Jan 7, 2014 at 10:46

4 Answers 4

9

The null-check in valueFirstGet can not yield true without there being undefined behavior in the program, because you must dereference a null-pointer (which causes UB) to let that test yield true. The nasty thing about undefined behavior is that any behavior is fully sanctioned and that behavior might change between compiler versions or even optimization levels without notice.

The correct way to go about this is to have the check in the caller or in a helper method. If the findObject/valueFirstGet calls are commonly paired with no further use for the object returned by findObject, you can even do something like this:

bool tryValueFirstGet(const std::string& key, int& value)
{
    Foo* obj=findObject(key);
    return obj && obj->valueFirstGet(value);
}

The short-circuit behaviour of && ensures that valueFirstGet will only be called if the object could be found.

2
  • Also this gives a gain in performance, since, in some cases i would know that I do not have a null pointer, and then the check is unnecessary. Using a helper method, gives the programmer the choice of guarded/unguarded behaviour.
    – user877329
    Commented Jan 7, 2014 at 11:03
  • I'd like to add that if you absolutely want to guarantee that you can call valueFirstGet without problems, then return a wrapper object which is never null and has a method valFirstGet which performs this check internally. However, if that is strictly the only reason for making a wrapper class, it's a waste of time and energy.
    – Neil
    Commented Jan 7, 2014 at 13:48
3

A common and useful way to avoid null pointer checking is by using the Null Object pattern. Essentially, instead of using the language's built-in definition of a null object as "a null pointer you can't dereference," you create your own definition with useful null behavior appropriate to your situation. Remember, you're a programmer. You don't have to settle for whatever limited facilities are built into the language.

Using the null object pattern for your example might look something like this:

class Foo {
  public:
    int getCount() {
      return n_values;
    }

    int firstValue() {
      return values[0];
    }
}

class NullFoo : public Foo {
  public:
    int getCount() {
      return 0;
    }

    int firstValue() {
      return 0; // Return a useful default if you want, or throw an exception if you want.
    }
}

Then if your findObject doesn't find the key, it instantiates and returns a NullFoo (or returns a reference to a static object), which can be dereferenced without undefined behavior, and can provide any kind of default behavior you like, including throwing an exception if the situation calls for it.

The nice thing is your regular Foo can be guaranteed to have at least one element, so you can skip a lot of checks there as well. All the checks are done at instantiation time, and polymorphism handles the rest.

7
  • This is a good technique in fully polymorphic language like Java, but in C++ polymorphism is often not the preferred approach. E.g. the virtuals are more likely slower than the null pointer check.
    – Jan Hudec
    Commented Jan 8, 2014 at 10:57
  • you misunderstood the question, and gave a wrong answer. In c++, this can never be nullptr. Commented Jan 8, 2014 at 12:22
  • while it is true that in C++ we often have better options than late -bound polymorphism, that does not invalidate this answer. The Null Object Pattern is very useful in C++, too. Commented Jan 8, 2014 at 13:12
  • 1
    @BЈовић, you misunderstood my answer. Using the null object pattern, this won't ever be a nullptr. It's a pointer to a valid object with default behavior you specify. Commented Jan 8, 2014 at 14:00
  • 1
    null object pattern is something else. this can never be nullptr, unless is set by a really bad programmer. btw changing this to point to something else is undefined behavior. Commented Jan 8, 2014 at 14:38
0

Summary:

The design problem you've presented actually deals with two kinds of issues, thus it involves two classes of possible solutions too.

Is it ok to let the member function do the null check instead of its caller?

Semantically speaking, it does not make sense. To call an instance method of a null object is logically incorrect because the object does not exist yet.

So that being said, a cleaner solution to this design problem would be to handle the case as clearly as possible by using the exception mechanism available in the language. Let the findObject throw the exception and let the caller routine handles it as needed. This results to a more readable code.

In my code, there are several calls to findObject directly followed by a call to valueFirstGet so leaving the check to the caller makes the code ugly.

This is where a wrapper method will come in handy to avoid repeating yourself.

-1

Sometimes, working with large chunks of legacy code, such null checking proves to be helpful. But I will never recommend it for a class being written from scratch.

Also note, that in valueFirstGet() the &value could also be null. I have worked with legacy code, where such situations had to be taken care of, too.

4
  • why and how are such checks helpful? Commented Jan 8, 2014 at 12:19
  • @BЈовић: because the C++ libraries I was porting from Win32 to Android NDK could not support exceptions, and they were almost cross-platform, only some objects were Windows-only, and happened to be NULL in Android. Few couples of added lines let us run the huge codebase without major rewrite.
    – Alex Cohn
    Commented Jan 8, 2014 at 13:53
  • as Bart wrote in his answer, this can never be nullptr, therefore the check makes no sense at all Commented Jan 8, 2014 at 14:35
  • Bart only wrote that before the null is sensed, you hit an "undefined behaviour". So far, so good. When you know exactly your target and your toolchain, you can know how they behave in this situation. Specifically, for Android NDK with gcc, a non-virtual method happily arrives to this != 0 clause. And believe me, there were tons of other undefined behaviours the library depended upon.
    – Alex Cohn
    Commented Jan 8, 2014 at 16:17

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.