Are getters and setters poor design? Contradictory

Question

This question already has an answer here:

• Why use getters and setters/accessors? [closed] 38 answers

I'm currently working on a simple game in Java with several different modes. I've extended a main Game class to put the main logic within the other classes. Despite this, the main game class is still pretty hefty.

After taking a quick look at my code the majority of it was Getters and Setters (60%) compared to the rest that is truly needed for the logic of the game.

A couple of Google searches have claimed that Getters and Setters are evil, whilst others have claimed that they are necessary for good OO practice and great programs.

So what should I do? Which should it be? Should I be changing my Getters and Setters for my private variables, or should I stick with them?

Answer 1

They absolutely are evil.

@coobird unfortunately they absolutely do not "enforce the concept of encapsulation", all they do is make you think you're encapsulating data when in fact you're exposing data via a property with delusions of method grandeur. Anything a getter/setter does a public field does better.

First, if you want public data, make it public, get rid of the getter & setter methods to reduce the number of methods the client has to wade through and make it cognitively simpler for the client to change it's value by eg.

object.field = value;

instead of the more cognitively intense

object.setField(value);

where the client must now check the getter/setter method to see if it has any side-effects.

Second, if you really need to do something else in the method, why call it a get/set method when it's got more responsibilities than simply getting or setting? Either follow the SRP or call the method something that actually tells you what the whole method does like Zarkonnen's examples he mentioned eg.

public void kill(){
    isAlive = false;
    removeFromWorld(this);
}

instead of

public void setAlive(boolean isAlive){
    this.isAlive = isAlive;
    if (isAlive)
        addToWorld(this);
    else
        removeFromWorld(this);
}

where does the setAlive(boolean) method tell the client that as a side-effect it'll remove the object from the world? Why should the client have any knowledge about the isAlive field? Plus what happens when the object is re-added to the world, should it be re-initialised? why would the client care about any of that?

IMHO the moral is to name methods to say exactly what they do, follow the SRP and get rid of getters/setters. If there's problems without getters/setters, tell objects to do their own dirty work inside their own class instead of trying to do things with them in other classes.

here endeth my rant, sorry about that ;)

Answer 2

It's a slippery slope.

A simple Transfer object (or Parameter object) may have the sole purpose of holding some fields and providing their values on demand. However, even in that degenerate case one could argue that the object should be immutable -- configured in the constructor and exposing only get... methods.

There's also the case of a class that exposes some "control knobs"; your car radio's UI probably can be understood as exposing something like getVolume, setVolume, getChannel, and setChannel, but its real functionality is receiving signals and emitting sound. But those knobs don't expose much implementation detail; you don't know from those interface features whether the radio is transistors, mostly-software, or vacuum tubes.

The more you begin to think of an object as an active participant in a problem-domain task, the more you'll think in terms of asking it to do something instead of asking it to tell you about its internal state, or asking it for its data so other code can do something with those values.

So... "evil"? Not really. But every time you're inclined to put in a value and expose both get... and set... methods on that value, ask yourself why, and what that object's reponsibility really is. If the only answer you can give yourself is, "To hold this value for me", then maybe something besides OO is going on here.

Answer 3

If you need external access to individual values of fields, use getters and/ or setters. If not, don't. Never use public fields. It's as simple as that! (Ok, it's never that simple, but it's a good rule of thumb).

In general you should also find that you need to supply a setter much less often than a getter - especially if you are trying to make your objects immutable - which is a Good Thing (but not always the best choice) - but even if not.

Answer 4

You may want to replace some of your classes by value classes. This will allow you to remove the getter and avoid problems when the content is changed from under you.