Getters/Setters and object oriented design? Is everything my school teaches wrong?

[BustTheCode]

Hey everyone, I took the time to read the stickies so I hope my first post here follows the guidelines.

I've just started learning c++ after transitioning from java, and came across this article - http://typicalprogrammer.com/doing-it-wrong-getters-and-setters/ - while researching c++.

This single article sent me on a long series of reading related articles because it seems very counter intuitive to what I was explicitly taught in my classes.

What are the uses of Getters and Setters? Are the uses different in c++ from java?

After reading a lot of similar articles, the logic seems to make sense behind not using them, but one article explicitly stated that getters and setters were not apart of object oriented design.

To ask a more specific question about some code I'm working on right now (just a very simple program to get acquainted with c++), I'm writing a very dumbed down employee database program. 3 classes, one class to store employee information, one to store a collection of employee objects, and one for the UI. If what I'm reading about getters and setters is correct, how should I approach my employee class? Would it be better to just make it's member variables public, or do I need to re-think my 3 classes altogether?

Thanks in advance!

[JLBorges]

Well, the idea is that a class encapsulates coherent behaviour and information; that means that it is (or ought to be) more than a container of sub-objects, each of which can be independently accessed in isolation.

A class representing a person (say, with a name, an address, and a mail id) should ideally be conceptually different from an array of three strings. (With name == 0, address == 1 and mail_to == 2).

There are classes that are nothing more than containers of sub-objects - for example std::vector<> or std::map<>. Saying that: 'getters and setters are not a part of object oriented design' is akin to saying: 'container classes are not a part of object oriented design'. As always, the truth lies somewhere in between; as always, we programmers have to evaluate the situation, and make an apt design decision.

[vlkon]

I am not too experienced but when I use setters/getters I try to make it in a way that they have some meaning.

One thing I use is declaring only getters without setters (setting is done by constructor). This way I can prevent changes to the value (the other option is to use const I think - as I said I am not an expert :)).

Example 1:

 Class Point2D{
public:
void setXY(int x, int y);
int getX();
int getY();

private:
int x, y;
};

This is a simple storage class to keep position information of a single point in 2D space. This way I can ensure that when I change position I have to make sure to specify both coordinetes (because I can easily forget that after changing X I also wanted to change Y but didn't).

Example 2:

 Class Temperature{
public:
void setT(float T, char unit);
float getT(char unit);

private:
float T;
};

This way I can store some numeric value (e.g. temperature). But I can use any unit I want (C, F, K, milliKelvin, some arbitrary unit...) and use setters/getters to recalculate it for different unit.
I can even add some checks - example "setting temperature [K] can't be lower than 0".

Yes, these are probably not the typical examples int getX() {return x;}; but for those it really seems a little pointless to force them private and then declare setter and getter.

[Disch]

To understand why getters/setters are bad, you have to understand what "encapsulation" is.

Encapsulation is the idea that objects represent a distinctive "thing". IE, they are a whole unit, and are not simply a collection of smaller parts. You interact with the class as if it were a singular unit. This is accomplished by making the separate parts private which disallows outside code from accessing them. And, ideally, even being aware of their existence.

Encapsulation works best when the inner workings of a class are completely unknown to outside code. This provides numerous benefits:

1) It makes the class easier to use because you don't have to know how it works to use it properly
2) It makes it harder to misuse the class because not being able to access individual members means you can't put the class in a bad state.
3) It isolates code which makes it easier to test, and makes it easier to find and fix bugs.
4) It allows the inner workings of a class to change. IE, anything private can be ripped apart and rewritten to be more efficient or allow for more functionality later. As long as you don't change the public interface, no outside code using the class needs to change or be updated.


Getters/setters go against (you could even say they completely destroy) the encapsulation mindset because to get/set something, you are extracting one of the class's individual parts. You are no longer treating it as a unit, but instead are treating it as a collection.


So yes... if your class was teaching you to write getters/setters for all your members... then your class was teaching you wrong. If every member has a getter/setter, then there's no value to the getters/setters. Your data might as well all be public.




No offense to vlkon, but his example of a Point2D class is the perfect example of something that should not be encapsulated. Or rather... it cannot be properly encapsulated. If you are treating a Point2D as a collection of X,Y components, then you might as well just make those components public. Writing getters/setters for them is unnecessary and is just extra work.

His temperature class is a much better example. The class itself is being treated as an individual unit (representing a temperature), and the interface allows for that temperature to be manipulated multiple ways.

[JLBorges]

> Encapsulation is the idea that objects represent a distinctive "thing".
> IE, they are a whole unit, and are not simply a collection of smaller parts.

This is simplistic to the point of being absurd. Objects model entities in the problem domain; and entities in real life quite often have replaceable parts. Object oriented programming certainly does not mean that an 'object-oriented' , 'encapsulated' car is so totally removed from reality that it just can't have a replaceable music system. Or that the car (not the owner of the car) decides what its music system should be.

The encapsulation of the standard stream classes are not violated because they provide getters and setters in getloc(), imbue() and the two overloads of rdbuf().


> Point2D class is the perfect example of something that should not be encapsulated. > ...
> then you might as well just make those components public.
> Writing getters/setters for them is unnecessary and is just extra work.

The only situation where thay should not be encapsulated are if there are no invariants associated with X and Y. Otherwise, interception and validation are still required. Even if there are no invariants, encapsulation allows alternate implementations without changing the interface.

An encapsulated

struct point2d
{
    // ...
    double X() const ; // getter
    void X(double) ; // setter
    double Y() const ;
    void Y(double) ;
    // ...
    
    private:
        double x = 0 ; // cartesian
        double y = 0 ; // cartesian
};

can, for performance reasons, be changed to

struct point2d
{
    // ...
    double X() const ; // getter
    void X(double) ; // setter
    double Y() const ;
    void Y(double) ;
    // ...
    
    private:
        double r = 0 ; // polar radius
        double phi = 0 ; // azimuth
};

without causing complete destruction of user code.

[Disch]

JLBorges wrote:

This is simplistic to the point of being absurd.

Of course it's simplistic, I'm not trying to write a novel on encapsulation. =P

I don't think it's absurd. I stand by it.

Objects model entities in the problem domain; and entities in real life quite often have replaceable parts. Object oriented programming certainly does not mean that an 'object-oriented' , 'encapsulated' car is so totally removed from reality that it just can't have a replaceable music system. Or that the car (not the owner of the car) decides what its music system should be.

A car is a fantastic example.

When you use a car, you are [mostly] completely unaware of its parts. You have a simplified interface (a wheel, pedals, some knobs/levers) and don't need to have any idea what is happening under the hood (figuratively and literally). What's more, whatever is under the hood can be swapped out entirely and the car can function the same as long as the interface doesn't change.


I'm not saying a class cannot have interchangable parts. I'm saying that when properly encapsulated, the programmer using the class does not need to concern himself with those parts. And the more parts that are exposed to him, the less encapsulated the class becomes (for better or for worse).

The encapsulation of the standard stream classes are not violated because they provide getters and setters in getloc(), imbue() and the two overloads of rdbuf().

I've shown some disdain for iostream in the past -- I am really not thrilled with a lot of its design choices.

But I'll refrain from getting too much into all that... and I'll agree with you somewhat. Having a getter/setter or two does not completely destroy encapsulation. Though is does expose its inner workings.

Understanding iostream enough to use rdbuf, etc requires you understand more than just the iostream class. There are couplings with streambuf you have to be familiar with... and a lot of internal functionality you have to understand.

So yes... I would say iostream is not fully encapsulated... as using its more advanced functionality does in fact require a deeper understanding of the underlying mechanisms.



On the other hand... 99.999% of the code that uses iostream sticks to the simplified interface and does not need to concern itself with those getters/setters.

[For a Point2D class] Even if there are no invariants, encapsulation allows alternate implementations without changing the interface.

Ehhh... your alternate implementation is going to perform like ass. If I write hundreds of lines of code where I use X() and Y() members to access the point data... then all of the sudden the underlying code changes on me and now all of those calls start calling multiple trig functions to convert behind my back... my program's performance will be destroyed.

This class should absolutely not be encapsulated. It is the wrong approach. Bottom line. I stand by that 100%.


A better approach would be to have 2 separate non-encapsulated structs with explicit ctors to allow converting between the two. Something like this:

struct Point2D;
struct PolarPoint2D;


struct PolarPoint2D
{
    double r;
    double phi;

    explicit PolarPoint2D(const Point2D& p);
};

struct Point2D
{
    double x;
    double y;

    explicit Point2D(const PolarPoint2D& p);
};

[dhayden]

I think the important thing is to understand the benefits of getters/setters and then to evaluate whether a particular circumstance will benefit from them. If it won't benefit then don't do it.

In other words, start with the the assumption that you *don't* need getters/setters, and then if there's a reason to use them, do it.

I find that they are overused. I see a lot of code at work where a class has members x,y, and z, and getters/setters for all three that just get/set the private member. The class is part of an internal system, so any code that uses it can be recompiled if the members change. The code has been the same for 15 years. In a case like this, the getters/setters are just in the way.

It may sound like I'm saying "don't use them" and I want to stress that this isn't the case at all. I've used them many times. What I'm saying is don't use them blindly.

[Disch]

In other words, start with the the assumption that you *don't* need getters/setters, and then if there's a reason to use them, do it.

I agree with this.

I think the problem is that people are taught "don't make your data public, instead write getters and setters" -- which is a horrible approach.

[48T7M4Gy]

There are compelling reasons for private class members and appropriate getters and setters.

"There are several benefits to be obtained from restricting access to a data structure to an explicitly
declared list of functions. For example, any error causing a Date to take on an illegal value (for
example, December 36, 2016) must be caused by code in a member function. This implies that the
first stage of debugging – localization – is completed before the program is even run. This is a special
case of the general observation that any change to the behavior of the type Date can and must
be effected by changes to its members. In particular, if we change the representation of a class, we
need only change the member functions to take advantage of the new representation. User code
directly depends only on the public interface and need not be rewritten (although it may need to be
recompiled). Another advantage is that a potential user need examine only the definitions of the
member functions in order to learn to use a class. A more subtle, but most significant, advantage is
that focusing on the design of a good interface simply leads to better code because thoughts and
time otherwise devoted to debugging are expended on concerns related to proper use.
The protection of private data relies on restriction of the use of the class member names. It can
therefore be circumvented by address manipulation (§7.4.1) and explicit type conversion (§11.5).
But this, of course, is cheating. C++ protects against accidjavascript:editbox1.editSend()ent rather than deliberate circumvention
(fraud). Only hardware can offer perfect protection against malicious use of a general-purpose language,
and even that is hard to do in realistic systems."
"The C++ Programming Language" Stroustrup 4th Ed

[Disch]

I should clarify my point.

I don't think getters and setters are always bad. Just in general practice they should be avoided. It's like goto... yes you can use it effectively and properly, but if you say "using goto is a good idea" to beginners, they're going to misuse it.

The same is true of getters/setters. If you tell a beginner that getters/setters are good, then they're going to write crap like this:

class Record
{
private:
    string name;
    string company;
    int age;

public:
    int getAge() { return age; }
    void setAge(int v) { age = v; }

    string getName() { return name; }
    void setName(string v) { name = v; }

    string getCompany() { return company; }
    void getCompany(string v) { company = v; }
};

This is a bad class. It is not encapsulated, and those getters/setters have absolutely no value at all.

I can't tell you how many times I've seen code like this posted on these boards.

[admkrk]

Thank Java for that.

[JLBorges]

> On the other hand... 99.999% of the code that uses iostream sticks to the simplified interface
> and does not need to concern itself with those getters/setters.

Formatted I/O relies on a bunch of getters and setters provided by the stream: setf, unsetf, precision, width, fill
And getters and setters to facilitate handling and reporting of I/O errors: clear, rdstate, setstate, exceptions

Granted, the getter and setter for exceptions is not all that common.
But 99.999% of the code that uses iostream does not need to format output or handle input errors?
Hmm.. So all the code that I've seen belongs to the minority 0.001%. One lives, and one learns.


> So yes... I would say iostream is not fully encapsulated...

The rest of the standard library is even worse:
smart pointers: as if get and reset / operator= were not enough, they also have release.
std::vector<> and friends: These are so badly designed that their entire interface consists of getters and setters.
...

Designed by a bunch of idiots who do not understand encapsulation 101.
They needed to have been clearly told: 'Getters/setters go against (you could even say they completely destroy) the encapsulation mindset'


> If I write hundreds of lines of code where I use X() and Y() members to access the point data...
> then all of the sudden the underlying code changes on me and now all of those calls start calling
> multiple trig functions to convert behind my back... my program's performance will be destroyed.

Performance is not governed by number of lines of code; it is governed by how many times a particular piece of code is executed. For instance, there needs to be just one line of code executed in a tight loop, that incrementally rotates the points that were set in an animation.

Encapsulation means that the users can still think of points as entities in a cartesian co-ordinate system, irrespective of the internal implementation details.
Encapsulation does not mean: no getters or setters; take it from me, they completely destroy encapsulation.


> I think the important thing is to understand the benefits of getters/setters and then to evaluate whether a particular
> circumstance will benefit from them. If it won't benefit then don't do it.

Yes. +1


> There are compelling reasons for private class members and appropriate getters and setters.

+1 to that too.



> This is a bad class. It is not encapsulated, and those getters/setters have absolutely no value at all.

This is a good class. It is encapsulated, it has getters and setters, those getters/setters have value.

class Record
{
private:
    string name;
    string company;
    int age;

public:
    int getAge() { return age; }
    void setAge(int v) { if( age < 0 ) throw std::logic_error("negative age") ; age = v; }

    string getName() { return name; }
    void setName(string v) { if( v.empty() ) throw std::logic_error("name can't be empty()"); name = v; }

    // ...
};

Tip: Use encapsulation (yes, encapsulation may involve providing getters and/or setters) if there are invariants to be established. Either right now, or if they may come into reckoning later, as the design evolves.
The quote from Stroustrup's 'The C++ Programming Language' in an earlier post gives the rationale.

[ne555]

> Well, the idea is that a class encapsulates coherent behaviour and information;
> that means that it is (or ought to be) more than a container of sub-objects,
> each of which can be independently accessed in isolation.
I'm not sure to follow what you mean by `container of sub-objects'.
With you `person' example, I understand like having an struct with no member functions and whose member variables may not interact

> There are classes that are nothing more than containers of sub-objects
> - for example std::vector<> or std::map<>
Then you put as an example `std::map'...
A container responsibility is to store and retrieve elements, fine.
The elements may have no other relationship that being in the same container.
But... the elements take (almost) no part in the implementation of std::map, you don't retrieve the root of the rb_tree or consult about the parent of the nodes.

Same for std::vector, you don't ask for `v.data()' to make a reallocation because you have reached the capacity.

¿so how `containers of sub-objects' should be interpreted?


> For instance, there needs to be just one line of code executed in a tight loop,
> that incrementally rotates the points that were set in an animation.
¿and how would you do that if your interface only have getX() and getY()?
Even if you do have get_r() and get_phi(), ¿how would the client know that they are better to use in a rotation? (as he doesn't known what your internal representation is)

Instead you could have a `rotate()' function that is coupled to the point.

And the abuse

p.use_polar_representation()
//loop
   p.set_phi( p.get_phi() + delta_phi );

then you have loosed the localization, as the logic is all over the client code.



Consider private getters and setters, xP

[JLBorges]

> ¿so how `containers of sub-objects' should be interpreted?
> The elements may have no other relationship that (than?) being in the same container.

As one would normally interpret them. A graph object would have graph operations; it would also provide getters/setters to add remove vertices and edges. A xml_tree would allow getting and setting of xml elements.

The composite design pattern is often used to represent polymorphic whole-part hierarchies. http://sourcemaking.com/design_patterns/composite
The design choice is about where (not whether) to place the child management getters/setters; Go4 has a discussion on the safety vs transparency trade-offs.


> ¿and how would you do that if your interface only have getX() and getY()?
> Instead you colud have a `rotate()' function
> then you have loosed...

Did the // ... both before and after the getters/setters suggest absolutely nothing to you?
Other than 'only have getX() and getY()'?

struct point2d
{
    // ...
    double X() const ; // getter
    void X(double) ; // setter
    double Y() const ;
    void Y(double) ;
    // ...
};

[MiiNiPaa]

Many questions abous "Should I use getters/setters" are void if you design classes beginning with public interface first. For example:

We want to have a Point class.
It should be constructible from either polar or cartesian coordinates and we should be able to read them. So we decided that we will have single constructor taking 2 doubles (coords) and enum telling if those coords are cartesian or polar. Said enum will be defaulted to cartesian. Also we will have get_x, get_y, get_r and get_p functions.
It should be immutable (aside from assigment). So no mutators at all.
It should be assignagnable, copyable and move constructible/assignable. Appropriate members declared
This is all for class and its members.
We would want to declare some utility non-member function, like move_point() which would return by value Point which is a point representing a result of sum of radius-vectors represented by 2 agrument points. For extra kicks it should be able to construct points in place by taking its constructor parameters (like emplace_back and make_* does).
And so on.

Then, when we know how user will be able to interact with our class we can select most efficient and appropriate implementation. Some methods can become trivial getters, some will have complex calculations and you cannot say that particular method is evil because it is a trivial setter. It is was just the most appropriate way to implement this class.

EDIT: forgot class interface:

class Point
{
  public:
    Point(double p1, double p2, point_type type);
    Point(const Point& other);
    Point(Point&& other);
    Point& operator=(const Point& other);
    Point& operator=(Point&& other);

    double get_x();
    double get_y();
    double get_r();
    double get_p();
  private:
    //What should we write here..?
};

[JLBorges]

> Many questions about "Should I use getters/setters" are void
> if you design classes beginning with public interface first.

Yes. +1.

[Disch]

Formatted I/O relies on a bunch of getters and setters provided by the stream: setf, unsetf, precision, width, fill

We clearly have different ideas of what getters/setters are.

[dhayden]

We want to have a Point class.
It should be immutable (aside from assigment). So no mutators at all.

I realize that this is just an example, but I wouldn't make such a limitation. When creating a class, it's hard to know how the class will be used down the road. I've found may occasions when an existing class has an unnecessary restriction that gets in the way of its usefulness.

My advice is to keep the class as open and flexible as possible without backing yourself into a corner.

[MiiNiPaa]

I would implement point class as immutable anyways.
Reasons:
1) Point is supposed to be lightweight class so creating new point instead of modifying old has no noticeable impact on perfomance.
2) If you change coordinate, it would be another point.
3) There is little reasons to modify, say, p. There is move_point which already covers 99% of reasons:

Point p = unit_pos(/*...*/);
double facing = unit_facing(/*...*/);
p = move_point(p, 5, facing + M_PI/6, pt::polar); //p is point 5 meters away and 30 degrees to the left of unit. 

EDIT: I failed to mention that Point is extramely widely used header only class. All member functions are inline, and utility functions (defined in separate header/cpp) does have invariant checking enabled by preprocessor defines.
Due to that, if we were to include setters in Point, we would not be able to do invariant checking, as point does not know context where it used (radius-vector or position). And even if we do, any change to error checking strategy would require recompilation of the entire project as opposed to a single file.

[LB]

@Disch your record class is well designed. You could make the functions virtual and have MemoryRecord, DiskRecord, DatabaseRecord, etc.