Which method is better for implementing get/set?

Question

There are two methos for implementing get/set.

Method 1:

Define get and set separately.

class my_class
{
  // ...
};

class main_class
{
public:

  my_class get_data() const
  {
    return m_data;
  }

  void set_data(my_class value)
  {
    m_data = value;
  }

private:

  my_class m_data;
};

Note: In this method get is fast enough: http://cpp-next.com/archive/2009/08/want-speed-pass-by-value

And another method is (Method 2):

Define two get bodies, First const and another non const.

class my_class
{
  // ...
};

class main_class
{
public:

  const my_class& get_data() const
  {
    return m_data;
  }

  my_class& get_data() // Works like set.
  {
    return m_data;
  }

private:

  my_class m_data;
};

Using these methods:

void main()
{
  main_class cls;

  // For method 1.
  my_class data;
  data = cls.get_data();
  cls.set_data(data);

  // For method 2.
  const my_class data1;
  my_class data2;
  data1 = cls.get_data();  // const get invoked.
  cls.get_data() = data2; // Like set beacuase non const get invoked.

}

My question which of these methods for implementing get/set is better?

Do you know a better method?

---

Edit: For answers that believe Method 1 is better, what do you say in below situation:

void main()
{
  main_class cls;

  // For method 1.
  cls.get_data().do_something_else(); // Not effictive for cls, because data losts.

  // For method 2.
  cls.get_data().do_something_else(); // Effictive for cls.    
}

Solution 1

You should always use a reference for any custom classes to pass just the address not the value class. You should also avoid passing back a non-const reference for editing. See below for my recommendations.

class my_class
{
  // ...
};

class main_class
{
public:

  const my_class & get_data() const
  {
    return m_data;
  }

  void set_data(const my_class & data)
  {
    m_data = data;
  }

private:

  my_class m_data;
};

Solution 2

I know this won't be a popular answer with C++ purists and before I learned Python, and Ruby I wouldn't have broached the possibility... but... Since the getter and setter you provided doesn't do range checking or special calculations why not make the member public?

 class main_class
 {
  public:
    my_class my_data;
 }

Sure, you'll lose the const on the getter and won't be guaranteed protection, but you're not guaranteed that anyway because you provide a set function, which modifies the member.

Solution 3

The second one is very bad as it abandons the encapsulation: you can as well just make the corresponding field public, anyone could access it without your object knowing about it. You cannot perform range checks or status updates based on the data being changed etc.

Solution 4

The second one would be a pretty bad choice. The reason for having setters is to be able to control how the member variable is modified by the user. If you just give the user a reference to your member, you lose all control.

So you're pretty much left with the first method. Below are two variations that you might or might not like:

// First Variation
// ---------------
// In this one both the setter and the getter have the same name
// (which is the same as the member they control). To get a
// variable you do `int i = foo.var()` and to set it you do
// `foo.var(6)`. 

class Some
{
  public:
    int var() const {
        return var_;
    }

    void var(int v) {
        var_ = v;
    }

  private:
    int var_;
};

// Second Variation
// ----------------
// You can also make the setter return a reference to `this`.
// This allows you to chain setters, which can _sometimes_ be
// more readable but it also has a few disadvantages.

class Employee
{
  public:
    Employee& salary(double dollars) {
        salary_ = dollars;
        return *this;
    }

    Employee& name(const string& n) {
        name_ = n;
        return *this;
    }

  private:
    double salary_;
    std::string name_;
};

// You can now do this...
Employee emp;
emp.name("John Barlick").salary(500.00);

// ... But this can become quite ugly if you chain a large amount
// of setters (you'd then probably have to break the lines in
// order to keep the code readable). It also is (technically)
// less efficient. 

// In case you have lots of setters you could probably do this:
// emp.name("John Barlick")
//    .salary(500.00)
//    .some(787);
//    .another('g');  

Solution 5

Usually getters/setters are defined:

  const my_class& get_data() const
  {
    return m_data;
  }

  void set_data(const my_class& _data)
  {
    m_data = _data;
  }

View more solutions

Comments

[Puppy]

Value is better. When rvalue references come around, the second form will invoke a copy when it could be a move, which is both inefficient and potentially incorrect depending on the type. Furthermore, such value copies are prime targets for compiler RVO/NRVO and don't typically require a human optimizer.

[Puppy]

However, it definitely could be improved by the use of a swap(). That would be a nice copy&swap.

[David Rodríguez - dribeas]

@DeadMG: Unless you are using C++0x (in which case you would use an rvalue-reference) the alternatives in this particular case are: pass by refence and make the copy internally, pass by value and swap for objects that are swappable. I have to agree with Armen here: there is no point in making your code bad for your current compiler just because it will be fast down the road --and after rewriting! Also, that exact piece of code demonstrates that the person that asked the question did not understand the article that is linked.

[David Rodríguez - dribeas]

+1, overall the real choices are either option one, with the setter that takes the argument and enables you to perform checks, or just make the member public. There is no point in making the member private just to write two methods that transform it to public.

[Jason]

Why should you avoid returning a non-constant reference for editing? The STL does that all the time ... plus your setter, since it involves a memory-copy operation, is going to be dog-slow if the my_class object is even remotely complex ... i.e., suppose my_class contains a single STL container ... you'd now be forced to copy the entire container twice every time you did a set-operation when you only needed to change a single object in the container. The first copy would be calling the getter to obtain an updated version of my_class, and the second copy would happen in the setter.

[Chris Snowden]

It's true that you do sometimes want to and should edit a non-const reference. But this is, for simple class types, the safest and most common encapsulation way to get / set.

[Greg Howell]

The goal here is to set the member, not to access its public interface. His setter involves no extra copying, it copies an address as a function parameter and then does an assigment. Returning a non const reference does one copy of the return value and one assignment.

[Jason]

First, the assignment in the setter is a copy. If you return a non-const reference, you are returning an address and then dereferencing that address ... there is no assignment of the entire class taking place. Secondly my_class is a class, you can't just assume it's "simple". As I noted, it could very well have a string, or a vector, or something else inside of it. Suppose you forget to get the latest version of m_data and then pass a my_class to the setter ... you have now just erased the entire contents of whatever was inside the STL container in m_data.

[nathan]

David, glad you agree. It took me a while to re-think the mandatory getters/setters that I was taught in college.

[Greg Howell]

@Jason The question explicitly asks for implementing get/set. Chris here gives IMO the standard implementation of a set method. It maintains encapsulation and does an assignment. It seems to me that you are arguing for a method that gives non-const access to a member so that you can access the member's interface. Such methods are fine, but they are not set methods in the context of this discussion.

[Jason]

@Greg ... okay, I understand the reasoning/need behind encapsulation here ... it's just that I think one needs to understand that these can be very heavy operations depending on the nature of the my_class object ... BTW @Chris, that edit was so I could give you a +1 vote -- I didn't change any content

[Paul Manta]

+1 For mentioning operator[] and -1 for mentioning performance, so the two cancel each other out. :P (If you're looking to make you code run faster, getters and setters are among the last places you should look...)

[Jason]

@Paul Thanks :-) ... although I do feel getters and setters can be an issue in regards to code performance. For instance, if you have to use an API rather than design one, then if the getters and setters are slow, you're stuck. So I think that those designing an interface, if they want it to be fast, need to take certain things into consideration before someone writes tons of code based on an interface that may have performance limitations.

[Bo Persson]

std::vector is a container that allows anyone to edit its contents. That is a special case and not a good interface design for other classes.

[Cody Gray]

This is not really a problem with modern compilers, though. Any trivial getter/setter method will be inlined for performance reasons. And now that Microsoft's C++ compiler supports link-time code generation (and presumably there is a GCC equivalent), it's irrelevant that the code is placed in the header file.

[Itsik]

You are forgetting that method one atleast gives you encapsulation, and in the future, may you add some checks, or change the internal implementation of your class, you won't break the api with your users.

[nathan]

I would argue that a getter/setter that are passthroughs, such as described in the question, provide only academic encapsulation. I agree with the point about possibly breaking an API, but the op didn't state the class was being used as an API interface.