Better to return a C++ reference or a weak_ptr?

Question

Suppose I have a class where I want the user to be able to have a reference to one of my members. Which is preferred?

class Member;

class ClassWithWeakPtr
{
private:
   boost::shared_ptr<Member> _member;
public:
   boost::weak_ptr<Member> GetMember();
};

or

class Member;

class ClassWithCppReference
{
private:
    Member _member;
public:
    Member& GetMember() {return _member;}
};

What do you think? When is one better than another?

Answer 1

My basic, ignorant guideline:

I'm realizing the latter can be unsafe if the instance of ClassWithCppReference may go away while someone still has a reference to a member. However, I can also see an argument for the latter class with POD types, such as say, a message to a device, and you didn't want to copy the member all the time.

Answer 2

Why not return a shared_ptr<>? Thatway the client gets to use what's returned for as long as he needs it, but there is no problem if the 'server' class goes away.

There are not too many situations where the semantics of weak_ptr<> make a lot of sense (caches and ???). Usually when a client asks for something, it want to have ownership of that thing determined by the client itself (and shared ownership is just as good as full ownership).

If you have a situation where the 'server' object can be destroyed without knowledge of the client (a situation where you might want to use weak_ptr<> or shared_ptr<>) about the worst thing you can do is return a reference to a member. In this case, the client can't know whether or not it's safe to access the returned reference. You have to return a copy of the member or a smart pointer that can correctly manage the lifetime of the member being returned.

Remember that in the case where there's an expression that produces a temporary ClassWithCppReference (which is not always obvious), the client that calls GetMember() won't even be able to use the returned reference in the next statement.

Answer 3

You should avoid giving away your internals; it's the guideline n. 42 of "C++ coding standards" (Herb Sutter and Andrei Alexandrescu). If for some reasons you have to, better to return a const reference and not a pointer because the constness does not propagate through it.

weak_ptr<> seems to be a viable solution even if its basic purpose is to avoid cycles of shared_ptr. Instead if you return a shared_ptr<> you extend the life of such internal which in most of the cases does not make sense.

The problem with the instance that goes away while someone handles a reference to its internals should be faced with a correct synchronization/communication between threads.

Answer 4

I think the only reasonable answer is, it depends on how Member is related to the Class, and what you want users of Class to be able to do. Does _member have an meaningful existence which is independent of the Class object? If it doesn't, then I don't think using a shared_ptr for it makes any sense, whether you return a weak_ptr or a shared_ptr. Essentially either would be giving the user access to a Member object that could outlive the Class object that gives it meaning. That might prevent a crash, but at the cost of hiding a serious design error.

As awgn indicates, you should be very careful about exposing your class internals. But I think there definitely is a place for it. For instance, I have a class in my code which represents a file object, composed of a file header object and a file data object. Completely duplicating the header interface in the file class would be silly and violate DRY. You could, I suppose, force the user to get a copy of the header object, make the changes to the copy, and then copy the external object back into the overall file class. But that's introducing a lot of inefficiency that only buys you the ability to make the file object representation of the header different than the header object representation. If you're sure that's not something you're going to want to do, you might as well return a non-const reference to the header -- it's simpler and more efficient.

Answer 5

I want to bring up something in response to the comments (from the OP and Colomon, mainly) about efficiency etc. Often times copying stuff around really doesn't matter, in terms of real performance.

I have written programs that do a lot of defensive copying. It's an idiom in Java, where, because all objects are passed by pointer, there's lots of aliasing going on, so you copy anything going into/out of your class to break the aliasing, ensuring that clients of your class cannot violate your class invariants by modifying an object after the fact.

The programs I've written have defensively copied whole structures in places, including entire lists and maps. Just to be sure that performance isn't affected, I profiled the program extensively. The main bottlenecks were elsewhere (and even then, I tuned those other places so that the main bottleneck left is the network). Nowhere did this defensive copying figure into the hot spots of the program.

---

ETA: I feel the need to clarify the point of my message, since one commenter read it differently from how I intended, and quite possibly others have done too. My point isn't that it's okay to copy stuff around willy-nilly; but rather, one should always profile the performance of their code, rather than guess wildly at how it will perform.

Sometimes, when copying whole structures still gives acceptable performance, and at the same time make the code easier to write, then in my opinion, that is a better tradeoff (in most cases) than code that's only marginally faster but much more complicated.

Answer 6

Depending on the context, either one could be fine. The main problem with returning a 'live' link to a member (if you have to expose one in the first place) is that whoever uses the exposed member is that your client might hold onto it longer than the containing object exists. And if your client accesses said member via a reference when its containing object goes out of scope, you'll be facing 'odd' crashes, wrong values and similar fun. Not recommended.

Returning the weak_ptr<> has the major advantage that it would be able to communicate to the client that the object they are trying access is gone, which the reference can't do.

My 2 pennies' worth would be:

• If none of your clients would ever use the member, you as the author are the only person to make use of it and control the object lifetimes, returning a reference is fine. Const reference would be even better but that's not always possible in the real world with existing code.
• If anybody else would access and use the member, especially if you are writing a library, return the weak_ptr<>. It'll save you a lot of grief and will make debugging easier.
• I would not return a shared_ptr<>. I've seen this approach and it is usually favoured by people who are uncomfortable with the concept of a weak_ptr/weak reference. The main downside I see with it is that it will artificially extend the lifetime of another object's member beyond the scope of its containing object. That is conceptually wrong in 99% of cases and worse, can turn your programming error (accessing something that isn't there anymore) into a conceptual error (accessing something that shouldn't be there anymore).

Answer 7

Returning a weak pointer will definitely be more expensive and serves no real purpose - you can't hang on to ownership for longer than the life of the main object anyway.

Answer 8

Why return a weak pointer? I think you are making it more complicated with no necessary benefit.

Answer 9

In most situations, I would provide

const Member& GetMember() const;
Member& GetMember(); // ideally, only if clients can modify it without
                     // breaking any invariants of the Class

weak_ptr< Member > GetMemberWptr(); // only if there is a specific need
                                    // for holding a weak pointer.

The rationale behind this is that (too me and probably many others) calling a method GetMember() implies that Class owns member and therefore the returned reference will only be valid for the lifetime of the containing object.

In most (but not all) code I have come across, GetMember() methods are generally used to do stuff with the returned member right away and not storing it away for later user. After all, if you need access to the member, you can always request it from the containing object at any time it is needed.