In this specific example, it really doesn't matter: your class is a mere data aggregation. One typical design error people make is to make their member variables private and then hurry to write accessors for each and every one of them.

That's not a good idea.

In fact, it only serves to make things worse.

Sure, they will argue that it allows them to later change how things work (encapsulation) without having to modify external code (e.g. to add parameter checking...), but in practice, they tend to provide many more accessors than are really needed, and end up exposing way too much of their class' implementation details.

Yes, even knowing that your class has a data field named 'age' is a piece of information you are revealing to the world.

A better practice is to expose operations on the class rather than those kind of get/set accessors, and to write client code in terms of those operations rather than in terms of the data in the class. The only things that really need access to your data fields are display functions (and even then...) and GUI builder tools (so that they can set the properties on each GUI component from within the GUI building application, and have the components immediately change). Most of your code shouldn't need to access those data fields directly, and you should consider making code that does part of your class.

As a small example, instead of letting people manipulate the age of your dog, you could store the date of birth at construction and compute the age upon request... (though you could use a private variable to cache the computation... people don't have to know - that's why it is private; having a set_age function pretty much implies there is an age variable).

edit: building on JimPrice's example, instead of a set_health() function, your tank class might have take_damage() and get_repaired() functions. get_speed() might work from a base_speed member variable, accounting from the damage the tank has suffered. Abstracting things that way mean you never have to explicitely reference a variable.

One final minor nit, I'm personally fond of this syntax, when accessors are necessary (and, yes, they sometime are, just not as often as people seem to believe):



Returning the object as a reference allows chaining: f.foo(10).bar(20);, in a manner very similar to chained assignment f = g = h; (which is why operator= and its variants +=, -= ... do return *this; by reference too).

Quote:Original post by Fruny
Making things private is thus a way to protect your class invariant, as well as hiding implementation details by forcing people to use a specified interface (Encapsulation).

This is a common explanation, but in my opinion, it is backwards and counter-productive.

First of all, you are not protecting the invariants -- you are protecting the user of the class. You are protecting the user from changes to the interface of the class. If the user wants to fiddle with implementation details and suspend (temporarily, of course) the validity of the class invariants, then more power to him. The real problem is that if the implementation of the class is exposed, then every time the implementation changes (even by the smallest amount), the interface changes, and that could have serious consequences.

Second, the reason for hiding details is not to force the user to use the class in a certain way. The reason is to clear the interface of distractions and complication. An interface that exposes implementation details requires the user to understand those implementation details in order to use the class. An interface should expose only what is necessary to use the class. Anything more just makes the class harder to use. Besides, nobody likes to be forced to do anything.

I'm growing tired of this quesion...
Read older forum topics, or [google]it, or read a book about it.

Please excuse my unusual grumpiness, I got horribly sun-burnt today!