3

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!

  • The problem is that you are storing a pointer and some magic needs to happen to make sure it stays valid. Just make m_bar a Bar instead of a Bar * and your problem disappears. Also a reference is preferable if nullptr is not a viable argument. – nwp Sep 19 '14 at 17:55
  • But copying an object is not the same as having some kind of reference to an object. Of course, I would always go for the copy if possible, but if Bar is an abstract class, this won't work. – Simon Lehmann Sep 19 '14 at 18:35
  • 2
    In that case take an std::unique_ptr<Bar>. Besides accepting a nullptr it clearly documents how the interface is to be used. – nwp Sep 19 '14 at 19:06
  • 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... – Simon Lehmann Sep 19 '14 at 20:37
  • Also, if the object you pass is not allocated with new but is allocated on the stack, a unique_ptr or shared_ptr is probably not a good idea. – Simon Lehmann Sep 19 '14 at 20:39
2

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.

  • 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. – Simon Lehmann Sep 20 '14 at 10:11
  • +1 useful view, although I do not quite see why a (raw) pointer would signal ownership transfer – Martin Ba Sep 21 '14 at 15:12
  • 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. – J Trana Sep 22 '14 at 0:41
0

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.

0

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.

-2

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

  • Sorry but this is bad advice on a lot of points. Review your coding guide. – stijn Sep 20 '14 at 7:11
  • 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. – Simon Lehmann Sep 20 '14 at 10:17
  • 1
    @stijn - of course actually telling people what is bad about these points might be much more interesting – Martin Ba Sep 21 '14 at 15:14
  • 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 – stijn Sep 21 '14 at 18:40

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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