Storing a pointer to an argument passed by (non-const) reference

Question

When designing an interface for passing objects which are meant to be stored for later use and which should not be 'null', I am always a bit uncertain if the argument should be passed by reference or as a pointer.

This is an example of what I mean:

class Foo
{
private:
    Bar* m_bar;

public:
    Foo() : m_bar(nullptr)
    {
    }

    void Register(Bar& bar)
    {
        m_bar = &bar;
        m_bar->Registered();
    }
    // -- OR --
    void Register(Bar* const bar)
    {
        if (bar == nullptr)
        {
            // Error!
        }

        m_bar = bar;
        m_bar->Registered();
    }

    // Some method makes use of the stored pointer later
    void DoSomething()
    {
        if (m_bar == nullptr)
        {
            // Error!
        }

        m_bar->DoOtherThing();
    }
};

My thoughts on this are:

1. The passed in reference may go out of scope before DoSomething gets called, but that may happen with the pointed to object as well.
2. Using the pass-by-non-const-reference version gets rid of duplicating the check for null and tells the caller that it is not possible to register 'nothing'.
3. It would be better to pass a reference in the constructor of Foo as it is required by DoSomething, but sometimes it this is not an option.
4. It would be better to pass a reference to DoSomething directly, but again, this is not always possible.

So if I need that kind of separate setter/register method, would it be clearer to use a reference or a pointer?

PS I know there is are two very similar questions Storing a pass-by-reference parameter as a pointer - Bad practice? and Reference vs dereference pointers in arguments C++/C, but I think they both are concerned with slightly different problems. The former deals mostly with the (ab)use of const and the latter does not say anything about storing a pointer. There may be another question/answer out there, if so, please just mark this a duplicate then!

Answer 1

Preparing my flame-retardant suit, as I feel some bias must be present in any answer to this question.

First, I would like to note that I work within the embedded world, so RAII and stack allocation are generally the way to go. In almost any circumstance, I see passing something in by reference as a good idea because of the obvious reason: it can't be NULL. But notably, this is because I primarily do stack allocation - so I'm not typically taking something from the heap and stuffing it into a reference when passing it in. This style of programming eliminates common sources of issues for concerns 1, 3, and 4 that you listed.

If I do pass in a pointer, it's for one of two reasons: 1) NULL really is an option (somewhat rare) or 2) I want to signal that my object is taking ownership of the one being passed in. I realize this comes down somewhat to style and domain, but it's a rule of thumb that's helped me make fewer errors and communicate with the programmers on my team about what my code is doing.

Answer 2

In a typical situation, you only pass a pointer if you want to accept nullptr or the typical usage of the type is not as a value. This is, for example, some (but not all!) interfaces. In most cases you always take a (potentially const) reference.

Answer 3

The problem in my mind is that whether you use a reference or a pointer, your class has a view into data that is created separately, may be destroyed separately, and the class has no way of knowing that.

Foo f;
if (something) {
    Bar x;
    // stuff with x
    f.Register(x);
}

f.DoSomething(); // screwed! 

It's not a good sign for such innocent code to do something so bad. It depends on the details of your intent which you haven't gotten into, but I'd probably just use a shared pointer. An alternative would be to have Bar's destructor reach into Foo and make the pointer null, but now Bar needs a pointer to Foo and it's getting a little silly. If you feel shared pointer doesn't mean your needs as an alternative to both solutions you considered, can you explain why?

Edit: let me briefly add that if in a similar situation to the one i outlined above, you do not want to extend lifetime as a shared pointer does, but simply to check validity and act appropriately, you could use a weak pointer, but that's starting to seem like overkill.

Answer 4

Our coding guide -- written partially by me :-) -- says to always pass by pointer in this case where you want to hold on to a pointer.

1. With pointers, you can mostly correctly use const and non const versions, whatever is more appropriate (i.e., you may only want to hold on to a const Bar*, in which case you then can also only pass a const Bar*) Using a const Bar& as parameter type is bound to result in a runtime crash sooner rather than later because:

   • a const ref binds -- silently -- to temporaries : holding to temporaries is obviously a bad idea
   • among other things, any implicit type conversion creates a temporary, so this will bite you even when you pass an lvalue that is of an implicitly convertible type.

2. If you state to always pass pointers, noone will get funny ideas and try to hold on to a constreference. (see bullet above)

3. The client side may be slightly more aware of potential lifetime issues when the interface involves a pointer, even if the pointer isn't always visible syntactically. That is, when I see an interface Register(Bar*) personally speaking, I'd be less surprised if it holds on to the pointer than if I see a Register(Bar&)

4. That "it should not be null" does, IMHO, not matter too much for this decision. Passing a pointer means that the function can actually check it, but again this may or may not be appropriate. (Maybe it's just an (normally unchecked) precondition violation.)