Best way to return an object in c++?

Question

I'm pretty noobish when it comes to c++, what is the better way of returning an object? I'm coming from the scripting world where Objects are always references, and am trying to achieve the same notion ... I'm basing this off of When to pass by reference and when to pass by pointer in C++?, where one user stated: "A good rule of thumb: "Use references when you can and pointers when you have to"."

// basic layer class
class Layer { private: Channel channel; // NEVER NULL };

// return object by pointer
Channel *Layer::getChannel() {
    return &channel;
};

// return by reference
Channel& Layer::getChannel() {
    return channel;
};

The problem with the second version is that the compiler will accept this line:

Channel channel = layer.getChannel();  // creates a copy BAD

when it should be:

Channel &channel = layer.getChannel();  // reference good

Is there any way to enforce a caller of the second option to force it to not create a new channel, or is the first option better anyways, even if it will never be NULL?

Answer 1

You need to adjust the Channel class itself so that it isn't copyable. If it is copyable, the user can copy it, and nothing you do can prevent it.

If copying is not a meaningful operation, then you can "disable" it. Simply define the copy constructor (Channel(const Channel&)) and the assignment operator (Channel& operator=(const Channel&)) to be private. Then any attempt at copying the class will result in a compile error.

On a side note, as others have mentioned, C++ is not the scripting languages you're familiar with. Everything is not a reference, and you're only setting yourself up for a world of pain by pretending otherwise. In C++, it is common to allocate objects on the stack, and pass objects by value, rather than passing references and pointers around.

Answer 2

Returning a reference (or const reference) is the normal way for a getter method to give the caller direct access to a member variable, so I'd recommend the second version of getChannel().

If you want to prevent callers from making inappropriate copies of Channel, you can accomplish that by making its copy constructor private. (If you want to prevent everything from making copies, even Channel itself, you can declare the constructor private and then not implement it.) But you should only do this if making a copy would actually be nonsensical, e.g. if the class represents some sort of underlying resource that can't be copied. Don't forbid copying just because you think the caller shouldn't need to; that's the caller's decision to make.

Answer 3

Return a copy of the object itself when copying isn't expensive for your purposes, and when you don't need to be able to change the original. This should be the default.

Channel Layer::getChannel() {     return channel; };

Return by reference or pointer when copying is expensive or when you might want to change the value. Returning by reference allows you do to things like this:

layer.getChannel().clear();

And have it act on the channel that's in that layer.

Returning a pointer is similar to returning a reference except that it gives you a little more flexibility, in that the pointer can pointer to no object at all. I often a pointer when I want to be able to use store the "channel" in another class. I'd then do

 class MyClass
 {
     // ...
     void setChannel(Channel *pC) { m_pChannel = pC; }
 private:
     Channel * m_pChannel;  // pointer to a channel that came from layer
 }

Answer 4

You can't stop a caller to create a new instance even when you use the pointer-return-version.

Channel* channel = new Channel(*layer.getChannel());

I know there is a way to achieve this goal. (For example, making Channle's ctor private so only it's static member function or its friend functions can create it.) However, I don't think this is the point of your question.

The point is that when you are making the member function returning either reference or pointer, you give a caller options he can choose whether he wants to copy it or reference it. Also, you can make your intention more clear by adding const to make it read-only.

For your case, I'd go for reference-return-version as the Channel cannot be null. If you do not want them to change the member variable, return const reference. Remember there is no single best way to decide return value type as it depends on what you want to say. Hope it helps! :)

Answer 5

Since you're returning a reference to the object, you are giving the users of the class direct access to the object, and if you're going to do that, why are you making the object private? Just make it public.

Answer 6

Most important is maintaining readability with the code that's around you. "When in Rome, do as the Romans do." is important. You write it once, but everyone who has to maintain your code has to read it. If all of a sudden your code follows different guidelines than everyone around you, that means they need to first figure out your style, then figure out what you're doing...

One approach I've seen work very well is having pointers for things you change and const references for things you don't:

class Passenger {
  ...
};

class Car {
public:
  int speed() const { return speed_; }
  void set_speed(int speed) { speed_ = speed; }
  const Passenger& passenger() const { return pass_;}
  Passenger* mutable_passenger() { return &pass_; }

private:
  int speed_;
  Passenger pass_;
};

Clients of this class can do:

const Passenger& pass = car.passenger();  // no copy, but don't need to deal with NULL ptrs.

Other answers suggesting making copying a compile error are good ones.