Are getters and setters poor design? Contradictory advice seen [duplicate]
Asked Answered
P

16

240

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?

Padron answered 19/2, 2009 at 12:33 Comment(1)
See also stackoverflow.com/questions/996179Unroof
Y
356

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; }
Yawning answered 19/2, 2009 at 13:3 Comment(8)
what about in a language like c# or actionscript, where get and set are part of syntax? isAlive could just have a getter but not a setter.Colonize
It's important to note here that validation can be done within these "action" methods.Devlen
I don't think the "alive" example is convincing. Instead of using "public void damage(int damage) { hp -= damage; }", one may use "public void damage(int damage) { hp -= damage; if (hp <= 0) setAlive(false); }" along with "public void setAlive(boolean alive){if (!alive) hp = 0;}", which still follows getter/setter pattern and makes more sense to me.Bedazzle
"using setters still breaks encapsulation by allowing you to set values that are meaningless." nonsense, you can validate the value to be set in your setter method thus can avoid meaningles valuesJuristic
This, 1k times over. Don't take the state data from the object, change it somewhere else, and put it back in. Give it all the data it needs to do the work itself. That's like OOP 101.Milt
This is great from a pure OO point of view. However, in the real-world, most projects would achieve the above with a Game POJO containing getters and setters. That gets sent to a GameService.updateGame method (probably containing a GameValidator to ensure that Game scores can only go up). This approach requires having getters and setters so that your POJO can be used as a transport object between application layers. I'm not saying I like it... but it's out there and seems relevant to the question...Charcot
As DanMan says, OOP 101 says, "Tell, don't ask." Tell the object to do a task for you and trust it to do it. Don't micro-manage it with sets() and gets() and do the job yourself, that's almost like having a dog and barking yourself.Protestant
"Don't take the state data from the object, change it somewhere else, and put it back in." - how does this mend with design patterns like the State Pattern for example?Wanton
R
185
  • Very evil: public fields.
  • Somewhat evil: Getters and setters where they're not required.
  • Good: Getters and setters only where they're really required - make the type expose "larger" behaviour which happens to use its state, rather than just treating the type as a repository of state to be manipulated by other types.

It really depends on the situation though - sometimes you really do just want a dumb data object.

Rost answered 19/2, 2009 at 12:36 Comment(14)
Since it's not obvious that your "somewhat evil" cases won't eventually need a "larger behaviour", i'd go for the "good" way, just in case, so you don't have to change your code everywhere if you change your mind !Garget
@Benoît - I think that's the point. Jon is saying that unnecessary getters and setters are barely better than public fields - there are better choices - but they are still better than public fields.Chlorosis
Each of these values change as the game goes on, so no constants are required. However, these variables are only really used for holding settings for the actual game (i.e. mode, timeLimit). This is why I cannot help but feel that they are somewhat unnecessary.Padron
Let me go a little bit futher. Should the internals of the class use getters/setters instead of directly accessing the fields? My opinion is that yes.Pusillanimity
@EnderMB: If the variables are meant to be settings, then encapsulate them in a settings class.Rost
So, would it be a good idea to create a separate Settings class, extend the Settings to the game, then extend the Game to its sister classes? If I do that am I still able to edit settings from within the sister classes?Padron
No, you don't want Game to extend Settings. Settings serve as a value object... a plain pojo with getters and setters.Mana
And as a bi-effect, you can then serialize and dump the settings class to a file, and you have saved your settings :)Seeseebeck
Let me see if I get this: Create a Settings class, put all my settings variables, as well as their getters/setters within this class, then call something like Settings s = new Settings() and then use s.getTimeLimit(). I'm struggling to understand from what I've read about value objects on Google.Padron
That sounds right to me.Burlburlap
@Mo - There's a lot to be said for a plain old pojo object :)Sulfaguanidine
Just don't forget to do a YAGNI check when tempted to promote a POJO to a Bean. Accessors, serialization, etc. are a form of complexity: https://mcmap.net/q/119313/-decoupling-vs-yagniPetition
@JonSkeet "make the type expose "larger" behaviour which happens to use its state, rather than just treating the type as a repository of state to be manipulated by other types." I have no idea what you mean by this. Please explain.Hartzell
@JanezKuhar: I mean that rather than making the caller perform multiple operations which manipulate the state in several steps but basically leave the type as a bag of state with no meaning, expose methods which perform meaningful operations which might manipulate (or at least use) those multiple pieces of state.Rost
Y
53

You've already had a lot of good answers on this, so I'll just give my two cents. Getters and setters are very, very evil. They essentially let you pretend to hide your object's internals when most of the time all you've done is tossed in redundant code that does nothing to hide internal state. For a simple POJO, there's no reason why getName() and setName() can't be replaced with obj.name = "Tom".

If the method call merely replaces assignment, then all you've gained by preferring the method call is code bloat. Unfortunately, the language has enshrined the use of getters and setters in the JavaBeans specification, so Java programmers are forced to use them, even when doing so makes no sense whatsoever.

Fortunately, Eclipse (and probably other IDEs as well) lets you automatically generate them. And for a fun project, I once built a code-generator for them in XSLT. But if there's one thing I'd get rid of in Java, its the over-dependence on getters and setters.

Yulandayule answered 19/2, 2009 at 13:26 Comment(9)
You left out the reason why they're evil.Announce
I fixed it so my objection is clearer.Yulandayule
ob.name = "Tom" may work the same as ob.setName("Tom"), but it's inferior because it doesn't allow you to add behavior later without changing the API. This is Object Oriented Programming 101.Toomay
@Jesse Barnum - I was aware that, in writing this, I was voicing a heterodox opinion. As we move into an increasingly multicore world, however, there is a growing number of us who find the Object Oriented model increasingly coming up short. It is, for one, an absolute mess to try to share state across multiple threads. For these and other reasons, I've recently become enamored of languages like Haskell, where immutability and the functional paradigm make it easier to get more done in less space. I'm not anti-Java, just frustrated by its limitations and its bureaucratic verboseness. YMMV.Yulandayule
In all my years of programming, I've never needed to change a getter or setter to do something differently. For a published API, yes to getters and setters. Within a project, where you can see all the field's uses in your IDE, a public field is just as good and less verbose, and you can easily use encapsulate field if you need specific behaviour.Tantalate
@Tantalate is right, I think it is a little bit of a misconception that we think the best place to add functionality (like "logging" or "validation" or "mutating") is at the setter. I think ideally we would do these sort of things somewhere else. For example, we would be more inclined to add logging to somewhere with meaning (ie. saving or emailing) instead of at a general setter. Or we may need to do validation based on context (ie. are we validating for saving? or validation for emailing?). Plus, there are many times where you want to validate everything together instead of individually.Colonnade
@Tantalate And C# basically follows that philosophy by allowing you to create a public field and later on change it to a property (although there are some small issues with that unfortunately).Stereotaxis
Excellent answer, and very well put. More support for this point of view at: Brainless Getters & Setters are a WasteMonaural
IMHO, most programmers use setter and getter because its auto generated (and is used for justify working hours on it). Is the same with the (used everywhere in some projects) Interfaces. Interface has a purpose, if the purpose is nil then don't use it.Jadajadd
Z
31

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 ;)

Zellazelle answered 8/10, 2011 at 11:3 Comment(3)
As an aside, think of a BankAccount object, would you prefer a setBalance() method or an increaseBalance() method?Zellazelle
Wanted to comment on Bill the Lizard's post...Zellazelle
To further add to this, it is a misconception to think the best place to add functionality (like "logging" or "validation" or "mutating") is at the setter. Ideally we would do these sort of things somewhere else. For example, we would be more inclined to add logging to somewhere with meaning (ie. saving or emailing) instead of at a general setter. Or we may need to do validation based on context (ie. are we validating for saving? or validation for emailing?). Plus, there are many times where you want to validate everything together instead of individually.Colonnade
T
30

Getters and setters enforce the concept of encapsulation in object-oriented programming.

By having the states of the object hidden from the outside world, the object is truly in charge of itself, and cannot be altered in ways that aren't intended. The only ways the object can be manipulated are through exposed public methods, such as getters and setters.

There are a few advantages for having getters and setters:

1. Allowing future changes without modification to code that uses the modified class.

One of the big advantage of using a getter and setter is that once the public methods are defined and there comes a time when the underlying implementation needs to be changed (e.g. finding a bug that needs to be fixed, using a different algorithm for improving performance, etc.), by having the getters and setters be the only way to manipulate the object, it will allow existing code to not break, and work as expected even after the change.

For example, let's say there's a setValue method which sets the value private variable in an object:

public void setValue(int value)
{
    this.value = value;
}

But then, there was a new requirement which needed to keep track of the number of times value was changed. With the setter in place, the change is fairly trivial:

public void setValue(int value)
{
    this.value = value;
    count++;
}

If the value field were public, there is no easy way to come back later and add a counter that keeps track of the number of times the value was changed. Therefore, having getters and setters are one way to "future-proof" the class for changes which may come later.

2. Enforcing the means by which the object can be manipulated.

Another way getters and setters come in handy is to enforce the ways the object can be manipulated, therefore, the object is in control of its own state. With public variables of an object exposed, it can easily be corrupted.

For example, an ImmutableArray object contains an int array called myArray. If the array were a public field, it just won't be immutable:

ImmutableArray a = new ImmutableArray();
int[] b = a.myArray;
b[0] = 10;      // Oops, the ImmutableArray a's contents have been changed.

To implement a truly immutable array, a getter for the array (getArray method) should be written so it returns a copy of its array:

public int[] getArray()
{
    return myArray.clone();
}

And even if the following occurs:

ImmutableArray a = new ImmutableArray();
int[] b = a.getArray();
b[0] = 10;      // No problem, only the copy of the array is affected.

The ImmutableArray is indeed immutable. Exposing the variables of an object will allow it to be manipulated in ways which aren't intended, but only exposing certain ways (getters and setters), the object can be manipulated in intended ways.

I suppose having getters and setters would be more important for classes which are part of an API that is going to be used by others, as it allows keeping the API intact and unchanged while allowing changes in the underlying implementation.

With all the advantages of getters and setters said, if the getter is merely returning the value of the private variable and the setter is merely accepting a value and assigning it to a private variable, it seems the getters and setter are just extraneous and really a waste. If the class is going to be just for internal use by an application that is not going to be used by others, using getters and setters extensively may not be as important as when writing a public API.

Tampon answered 19/2, 2009 at 13:27 Comment(6)
No you can just keep the setter **private ** anyway. Immutability is an other topic, by definition imuttable is when you can't change it's value after created like String for example. So this in your example it's a bug Look at C# or language supoorting properties.Sarcoid
@Simo - the backing array is immutable in the example.Inocenciainoculable
@Tampon - even if the getters/setters are simply returning/setting with no additional logic, do they not future proof the API when additional logic might be needed?Inocenciainoculable
@wulfgar.pro: Yip, they sure do, and that's one of the advantages of using setters and getters.Tampon
Getters and setters do not enforce anything. javaworld.com/javaworld/jw-09-2003/jw-0905-toolbox.html. Good encapsulation == your API goes after some real business logic not after row data. For instance object Car may have method: accellerate(Integer spead) { sets speed and do 10 other things - good encapsulation} or setSpeed(Integer speed) { //just setter - poor encapsulation}.Unreflecting
Adding logic to the encapsulation is evil. Sometimes is a needing evil but its still evil. If i have a function named "setValue", then its about setting a value and not other task.Jadajadd
B
19

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.

Bureaucrat answered 19/2, 2009 at 13:2 Comment(0)
D
15

Your Game class is probably following the god object antipattern if it exposes that many variables. There's nothing wrong with getters and setters (though their verbosity in Java can be a bit annoying); in a well-designed app where each class has a clearly separated functionality, you will not need dozens of them in a single class.

Edit: If the main point for the getters and setters is to "configure" the game classe (I understand your comment that way), then your probably don't need the getters (it's perfectly fine for a class to access its own private variables without using get methods), and you can probably collapse many of the setters into "group setters" that set several variables which belong together conceptually.

Diaconal answered 19/2, 2009 at 12:43 Comment(3)
The problem is that whilst there are several settings for this game they are minute in their differences, with only a couple of extra methods typically needing to be applied in the sister classes to provide different values.Padron
Thanks for the reply. In response to your edit, if I call two setters from within every sister class it would be good OO practice to remove both setter methods in the main Game class to turn then into one group setter?Padron
Exactly - if those values are always set together, it's better to convery this information by having only one setter. It would be even better to put them together in a class, if there are more than two, or if there are methods that operate only on them and which could also be put into that class.Diaconal
D
11

The presence of getter and setters tends to indicate (a "smell" if you are into that sort of primary school language) that there is a design problem. Trivial getters and setters are barely distinguishable from public fields. Typically the code operating on the data will be in a different class - poor encapsulation, and what you would expect from programmers not at ease with OO.

In some cases getters and setters are fine. But as a rule a type with both getters and setters indicates design problems. Getters work for immutability; setters work for "tell don't ask". Both immutability and "tell don't ask" are good design choices, so long as they are not applied in an overlapping style.

Danicadanice answered 19/2, 2009 at 12:56 Comment(2)
Trivial getters/setters have a significant advantage to non-private fields -- their implementations can be changed later without affecting callers. This allows controlled access/mutation where needed.Chausses
It generally doesn't really make any sense to change their implementation.Danicadanice
D
10

My opinion is that getters and setters are a requirement for good programs. Stick with them, but don't write unnecessary getters/setters - it's not always necessary to directly deal with all variables.

Dionysus answered 19/2, 2009 at 12:36 Comment(0)
P
9

I don't really think they are evil. But I would love to live in a world where I never had to use them unless I really needed to.

One example I read above was future-proofing your code. For example:

public void setValue(int value)
{
    this.value = value;
}

Then, the requirements change and you need to track how many times the value was set.

So:

public void setValue(int value)
{
    this.value = value;
    count++;
}

This is beautiful. I get it. However, in Ruby, would the following not serve the same purpose?

someobject.my_value = 100

Later, you need to track the number of times my_value was set. Well then, could you not just override the setter THEN and only THEN?

def my_value=(value)
    @my_value = value
    @count++
end

I'm all for beautiful code but I have to admit, looking through the mountains of Java classes we have and seeing literally thousands and thousands of lines of code that are NOTHING but basic getter/setters is ugly and annoying.

When I was developing in C# full time, we used public properties all the time and did custom getters/setters only when needed. Worked like a charm and it didn't break anything.

Psychoneurosis answered 3/1, 2012 at 16:20 Comment(2)
You can have the same thing in Java if you use Lombok. IDE support for lombok exists for IntelliJ, Eclipse, and Emacs.Chouinard
I consider overriding a setter is even more evil. Users of the API consider a setter to change just one variable - and not to do complex things. The "correct" way would be to rename the setter in a way that later changes do not destroy user expectations.Chilly
R
4

As always the only answer is: it depends. If you are the only peron touching the code, you can do anything you're comfortable with, including taking shortcuts.

One of the benefits of using setters is that checks need to be performed at only one location in your code.

You might want to pay some closer attention to what is actually being get and set by these methods. If you're using them to provide access to constant values you are probably better off by using constants.

Rice answered 19/2, 2009 at 12:38 Comment(0)
C
3

This depends on the programming language in question. Your question is framed in the context of Java, where it seems that getters and setters are generally thought of as a good thing.

In contrast, in the Python world, they are generally considered as bad style: they add lines to the code without actually adding functionality. When Python programmers need to, they can use metaprogramming to catch getting and/or setting of object attributes.

In Java (at least the version of Java I learned slightly a decade ago), that was not possible. Thus, in Java it is usually best to use getters and setters religiously, so that if you need to, you can override access to the variables.

(This doesn't make Python necessarily better than Java, just different.)

Charlacharlady answered 19/2, 2009 at 12:53 Comment(0)
A
2

Just FYI: In addition to all the excellent answers in this thread, remember that of all reasons you can come up with for or against getters/setters, performance isn't one (as some might believe). The JVM is smart enough to inline trivial getters/setters (even non-final ones, as long as they aren't actually overridden).

Agiotage answered 19/2, 2009 at 12:59 Comment(0)
L
1

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.

Laktasic answered 19/2, 2009 at 12:37 Comment(0)
C
1

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.

Chlorosis answered 19/2, 2009 at 12:42 Comment(0)
W
0

I've been programming in java for few monts ago, and I've learned that we should use getters & setters only when it's necessary for the application

have fun :)

Widescreen answered 19/2, 2009 at 13:40 Comment(4)
What if your object changes so that one field needs a getter and a setter? If that field is being used in 100 different places, that's 100 different changes you need to make. If you use getters and setters from the start you only need to change it in one place, the object that's changing.Announce
If you need to expose that variable to 100 different places, use a getter and setter. But is it really good OO practice to have 100 different places playing with your object's internal data?Transeunt
@Terry: That object could be used in 100 different projects, not just called in 100 different places in one project.Announce
...also, I should be saying "class" instead of "object".Announce

© 2022 - 2024 — McMap. All rights reserved.