Follow the link or create a typical setter / receiver?

Question

I was interested to learn about good practices in C ++, and I ran into the problem of creating getter / setter for a class member.

So, why not just return the element by reference, so that I can change or access its value to read it? In particular, this is my code:

class Chest : public GameObject { public: Chest(); ~Chest(); static int& num_chests(); private: static int num_chests_; }; 

Is this a bad practice? Should I use them instead?

 class Chest : public GameObject { public: Chest(); ~Chest(); static int num_chests(); static void set_num_chests(int num_chests); private: static int num_chests_; }; 

Answer 1

If you are not very opposed to this, use the functions of the receiving member and setter.

The reason int& num_chests() or the public field is bad is because you are binding client code that uses the num_chests value and the fact that it is actually a field (internal implementation detail).

Suppose you later decided that you would have your own std::vector<Chest> chests in your class. Then you do not want to have an int num_chests field - this is terribly redundant. You would like to have int num_chests() { return chests.size(); } int num_chests() { return chests.size(); } .

If you use a public field, now all your client codes should use this function instead of the previous access to the field - each use of the num_chests value num_chests be updated as the interface has changed.

If you used the function that returns the link, now you have a problem, because chests.size() is a return by value - you cannot in turn return this by reference.

Always encapsulate your data. This requires only a minimal amount of template code.

In response to comments saying you should just use public fields:

Keep in mind that the only advantage of using open fields (other than the remote possibility of some micro-optimization) is that you do not need to write template code. "My teacher hated it when I used public fields (and he was annoyingly annoyed)" is a very bad argument for using public fields.

Answer 2

In almost all cases when you think you need to create a setter AND getter (that is, both at the same time), your design is wrong.

Think about the purpose of num_chests ? You cannot go anywhere without knowing what it is.

According to your code, I assume that it contains the number of chests per level. In this case, you do not want to provide a setter for this value for everyone. You want the value to be equal to the number of chests in the game, and by providing a setter, anyone can annul this invariant.

Instead, you can provide ONLY a getter, and you can control its value in your class.

 class Chest : public GameObject { public: Chest() { ++num_chests_; } ~Chest() { --num_chests_; } static int num_chests() { return num_chests_; } private: static int num_chests_; }; int Chest::num_chests_ = 0; 

More explanations on why getters and setters are the wrong decision in my opinion. If you provide setter and getter, you only have the illusion of control over a variable. Consider std::complex . Why

 std::complex<double> my_complex(1.0, 5.0); my_complex.real(my_complex.real()+1); 

it's better

 std::complex<double> my_complex(1.0, 5.0); my_complex.real()++; 

Answer: when std::complex was developed, there were no references to C ++. In addition, Java has no C ++ references, so they must write boilerplate code everywhere. Now GCC returns non-constant links here as an extension, and C ++ 11 allows

 reinterpret_cast<double (&)[2]>(non_const_non_volatile_complex_variable)[0] 

and

 reinterpret_cast<double (&)[2]>(non_const_non_volatile_complex_variable)[1] 

as a valid way to access the real and imaginary parts of std::complex<value> .

Answer 3

The purpose of your interface does not have to be the simplest for the program , but the simplest version is also used . .

If you are unable to provide the setter and getter methods, you are setting yourself up for later headaches. For example:

• what happens if you need to be notified when someone changes the num_chests value?
• What happens if you need to confirm that num_chests cannot be negative?
• What happens if you need to run the program in a multi-threaded environment and you need to block reading until the records are ready?

As you can see, an interface that is transparent to the user is also easier to protect against user errors, as well as spread in the future; this advantage brings a very small (if any) additional cost.

On the other hand, sometimes you want to return a link or pointer to an internal member. For example, container classes in the standard library often offer a data() method that retrieves a pointer to the base container (both in const and non const variants).

So, this is not a strict rule, but I would say that returning non-constant references to private members hits the goal of OO programming.

Answer 4

I strive for low adhesion and high cohesion; I avoid getters and setters.

If you have receivers and setters, another object will need to know what to call them. This connection.

Try to separate your objects, but make them cohesive. By cohesion, I mean that they work well with the rest of the system.

What is your system? Why do you have getters and setters? Because you want to control and display these objects. They are a model, and you have controllers and views on them.

It's easy to fall into the trap of communication between your control / presentation and your model.

To avoid communication, let the model create a control and update the view. Then he should not have any getters or setters.

eg.

 struct Instrumenter { virtual void addRange(int& value, int min, int max) = 0; }; struct Renderer { virtual void render(std::string& description, int value) = 0; }; struct GameObject { virtual void instrument(Instrumenter& instrumenter) = 0; virtual void display(Renderer& renderer) = 0; }; struct Chest : GameObject { virtual void instrument(Instrumenter& instrumenter) { intrumenter.addRange(count, 0, 10); } virtual void display(Renderer& renderer) { renderer.render("Chest count", count); } private: int count; }; 

Then you can use it like this:

 int main() { vector<shared_ptr<GameObject>> gameObjects; MyControlInstrumenter instrumenter; // ... for(auto& gameObject: gameObjects) { gameObject->instrument(instrumenter); } // etc. }