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!

• nwp: In that case take an std::unique_ptr<Bar>. Besides accepting a nullptr it clearly documents how the interface is to be used.
• Simon Lehmann: Ok, a unique_ptr is a nice thing if you want to transfer ownership, but what if you don't want to give up ownership of an object you pass here? You may suggest shared_ptr, but it still allows for null...
• nwp: If the object is on the stack a pointer is a bad idea no matter what type. Making the pointer owning and not owning and not null and work properly for any object on the heap and on the stack is just not possible. You will have to figure out something that makes sense with the specific types, goals of the API and consistency with the rest.
• D Drmmr: @SimonLehmann You find it unexpected that the lifetime of a local variable is tied to its scope? Really? If you need to extend the lifetime of an object beyond its scope you need to allocate it on the heap. That's C++ 101.
• Simon Lehmann: @DDrmmr No, I find it unexpected if someone passed me a shared_ptr which will later point to a deallocated object.

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.

• Simon Lehmann: Interestingly, the project I am currently working on is also in an embedded system, so the answer may be depend on that. This is also the reason why things like std::unique_ptr are not always possible.
• Martin Ba: +1 useful view, although I do not quite see why a (raw) pointer would signal ownership transfer
• J Trana: Ah - it's just a useful convention. If you use a reference, you really have to go out of your way to delete it, so you can make a good guess you don't own it. To make it very black/white, a pointer indicates the opposite - though sometimes exceptions are necessary and then you comment it up with a big note.

Answer 2

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 3

In my C++ source code, there is no indication whether I pass an argument by value, by constructing reference, or by non const reference. From the language point of view, there is no difference is that in the first case the argument is not modified, in the second case it can be modified by jumping through nasty hoops and risking undefined behaviour, in the third case it can easily be modified. In neither case there is any indication that a pointer to the argument could be stashed away somewhere which means someone must keep the object alive.

So this is totally unexpected to the caller. It is a bit less unexpected if you pass a pointer. Anyway, a pointer should only be stored if the argument was stack allocated and responsibility for the object is passed somewhere else.

Answer 4

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 5

If Foo can't do its job without a Bar (or even a registered Bar) then don't make the Foo until the Bar is available. In the typical case where the Bar is only needed for the Foo, make it during construction of Foo.

If Foo can do its job without a Bar, then pass the optional Bar as a parameter to DoSomething.

Answer 6

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.)

• stijn: Sorry but this is bad advice on a lot of points. Review your coding guide.
• Simon Lehmann: While I feel this is not what I would use as a coding guide, this is exactly what I am trying to get at: How may others see and interpret an API which uses either pointers or references. But I think const vs. non-const references make a huge difference, which is why I linked to the other question about that. However, it would be an interesting question if I actually wanted to store a pointer to a const something.
• Martin Ba: @stijn - of course actually telling people what is bad about these points might be much more interesting
• stijn: that's a bit too much for a comment; here's some: With pointers, you can use const and non const versions idem for references - temporaries and point 2 pointers can point to out of scope objetcs as well - 3 sort of agree, but 'slightly more aware' doesn't really cut it, lifetime issues should be obvious or properly documented - but I rarely do this, possibly just via an assert. I'd argue inputs should always be checked instead of crashing later on