Are Getters and Setters Poor Design? Contradictory Advice Seen

Are getters and setters poor design? Contradictory advice seen

There is also the point of view that most of the time, using setters still breaks encapsulation by allowing you to set values that are meaningless. As a very obvious example, if you have a score counter on the game that only ever goes up, instead of

// Game
private int score;
public void setScore(int score) { this.score = score; }
public int getScore() { return score; }
// Usage
game.setScore(game.getScore() + ENEMY_DESTROYED_SCORE);

it should be

// Game
private int score;
public int getScore() { return score; }
public void addScore(int delta) { score += delta; }
// Usage
game.addScore(ENEMY_DESTROYED_SCORE);

This is perhaps a bit of a facile example. What I'm trying to say is that discussing getter/setters vs public fields often obscures bigger problems with objects manipulating each others' internal state in an intimate manner and hence being too closely coupled.

The idea is to make methods that directly do things you want to do. An example would be how to set enemies' "alive" status. You might be tempted to have a setAlive(boolean alive) method. Instead you should have:

private boolean alive = true;
public boolean isAlive() { return alive; }
public void kill() { alive = false; }

The reason for this is that if you change the implementation that things no longer have an "alive" boolean but rather a "hit points" value, you can change that around without breaking the contract of the two methods you wrote earlier:

private int hp; // Set in constructor.
public boolean isAlive() { return hp > 0; } // Same method signature.
public void kill() { hp = 0; } // Same method signature.
public void damage(int damage) { hp -= damage; }

Getters and Setters are bad OO design?

Getters or setters by themselves are not bad OO design.

What is bad is coding practice which includes a getter AND a setter for EVERY single member automatically, whether that getter/setter is needed or not (coupled with making members public which should not be public) - because this basically exposes class's implementation to outside world violating the information hiding/abstraction. Sometimes this is done automatically by IDE, which means such practice is significantly more widespread than it's hoped for.

Getters and Setters are bad OO design?

Getters or setters by themselves are not bad OO design.

What is bad is coding practice which includes a getter AND a setter for EVERY single member automatically, whether that getter/setter is needed or not (coupled with making members public which should not be public) - because this basically exposes class's implementation to outside world violating the information hiding/abstraction. Sometimes this is done automatically by IDE, which means such practice is significantly more widespread than it's hoped for.

Using accessors: Good or bad?

Looking at a short code snippet will naturally appear simplistic and wouldn't call for such encapsulation. As program size grows, I can see this approach saving time during maintenance. I don't have much experience with this approach, so I am unable to give a definite answer, but I think it can provide enough benefit to be worth trying and measuring the results.

For readability, I think there are some advantages to this approach over getters/setters. Assume you want to show the employee's work schedule as a table. Using the "builder" approach, the high-level code remains mostly unchanged:

ScheduleDisplay display = new TableScheduleDisplay();
employee.Export(display);
display.Render();

This is much easier to read than a list of instructions whose main purpose is to pull data from an object and place it into another object. I know at a glance that this code displays the schedule as a table.

Are getters and setters poor design? Contradictory advice seen

There is also the point of view that most of the time, using setters still breaks encapsulation by allowing you to set values that are meaningless. As a very obvious example, if you have a score counter on the game that only ever goes up, instead of

// Game
private int score;
public void setScore(int score) { this.score = score; }
public int getScore() { return score; }
// Usage
game.setScore(game.getScore() + ENEMY_DESTROYED_SCORE);

it should be

// Game
private int score;
public int getScore() { return score; }
public void addScore(int delta) { score += delta; }
// Usage
game.addScore(ENEMY_DESTROYED_SCORE);

This is perhaps a bit of a facile example. What I'm trying to say is that discussing getter/setters vs public fields often obscures bigger problems with objects manipulating each others' internal state in an intimate manner and hence being too closely coupled.

The idea is to make methods that directly do things you want to do. An example would be how to set enemies' "alive" status. You might be tempted to have a setAlive(boolean alive) method. Instead you should have:

private boolean alive = true;
public boolean isAlive() { return alive; }
public void kill() { alive = false; }

The reason for this is that if you change the implementation that things no longer have an "alive" boolean but rather a "hit points" value, you can change that around without breaking the contract of the two methods you wrote earlier:

private int hp; // Set in constructor.
public boolean isAlive() { return hp > 0; } // Same method signature.
public void kill() { hp = 0; } // Same method signature.
public void damage(int damage) { hp -= damage; }

Why do we actually use getters and setters?

Getters and Setters are abstraction wrappers around your object implementations. You may have a property of an object that is actually (as an example) computed based on the value of other field members, but not stored or persisted within the class, thus a getter creates the illusion that the property itself is native to the class, while its implementation is substantially different. Also, getters without setters allow you to create read-only properties and protect internal members of the class from inadvertent and possibly destructive modification.

It's all about separating the external representation consumed by other applications from the internal implementation plumbing. Those are the basics.

How to NOT use getters and setters

It's the second article I read from JW in less that three days and they both share the Why XXXXX is/are evil motto... leaving that behind I think I'll quote the final words of the article and give my opinion about it.

You shouldn't use accessor methods (getters and setters) unless absolutely necessary because these methods expose information about how a class is implemented and as a consequence make your code harder to maintain. Sometimes get/set methods are unavoidable, but an experienced OO designer could probably eliminate 99 percent of the accessors currently in your code without much difficulty.

I believe the author refers to concrete objects, like POJOs (why not entities) that do not interact with anyone. Your site has a login feature, so you have an user... why should you bother with getName and setName if it's just a String?

While this can be true, what happens if a name is null? Are you exposing yourself to NullPointerExceptions all around your code? Unless you do a check everytime you use that variable or you make sure no user has a null name you can never be sure. A getter can make it easy, but IMHO it's all about taste. You're just moving a check from one place to another.

Getter/setter methods often make their way in code because the coder was thinking procedurally. The best way to break out of that procedural mindset is to think in terms of a conversation between objects that have well-defined responsibilities. Cunningham's CRC card approach is a great way to get started.

Here, the author seems to be diggin further in the principle of CRC (explained in the article itself), which is a valid argument against most accessors.

Again, it's all about knowing your application and what will you let the others know about it. I can't think of an alternative right now, but instead of alternatives I prefer to dig into the concept of Cost VS Benefit, what's the benefit of avoiding a getter and how much does avoiding it cost to you (time to understand a variable, time to do checks, time to find the field, time to determine and handle the impact of a change where there's no accessor to limit it).

Another interesting point (and now that I think of it, one of the most important) is WHEN to avoid getters and setters. They're obviously evil if you expose something you're not supposed to, like some important variable that holds a counter and can leave your application in an inconsistent state (in other words... "go insane"). In those cases, a getter suffices.

Finally, a lot of frameworks and tools rely in those accessors to set and get values. Avoiding them means losing "compatibility" with those tools... unless you create indirect accessors or wrappers, for example.

---

Related Topics