Noooo!!! Don't click that generate getters and setters option!!!
I like the rule: "Don't use accessors and mutators." Like any good rule, this one is meant to be broken. But when?
First, let me be clear about what I am saying: adding getters and setters to OO classes should be the last resort after considering a series of better alternatives. I believe that a careful analysis yields the conclusion that getters and setters are harmful in the clear majority of cases.
What is the harm?
First let me point out that the "harm" I am talking about might not be any harm at all. The following is, in some cases, a perfectly reasonable class:
// Car1.java
public class Car1 {
public Engine engine;
}
Notice, though, that feeling of tightening in your stomach, the bristle of your hair, the tensing of the muscles that you may experience looking at something like that.
Now, I want to point out that there's no meaningful functional difference between that class, a public class with a public class member, and the following class below, a public class with a private member that is exposed by getters and setters. In both classes, Car1.java and Car2.java, we get essentially the same result.
// Car2.java
public class Car2 {
private Engine engine;
public Engine getEngine() {
return engine;
}
public void setEngine(Engine engine) {
this.engine = engine;
}
}
To show this, I read and write the engine in either Car1.java or Car2.java:
// Car1 member read and write
Car1 car1 = new Car1();
logger.debug("Car1's engine is {}.", car1.engine);
car1.engine = new HemiEngine();
// Car2 member read and write
Car2 car2 = new Car2();
logger.debug("Car2's engine is {}.", car2.getEngine());
car2.setEngine(new HemiEngine();
The point here is that anything I can do with Car2.java, I can do with Car1.java, and vice-versa.
This is important because we've been taught to get squeamish when we see Car1.java. We see that public member sitting there and we say, not safe! Engine is not protected by anything! Anyone can do anything! Aaaaaaaaagggghhhhh!
Yet for some reason we breathe a sigh of relief when we see Car2.java. Which--I'm sorry--I personally think is funny since there's literally the same (non-existent) protections around both of these things.
What could go wrong?
The following are some of the disadvantages of public getters and setters that directly expose a single private member, have the same name, and provide no other functionality.
Getters and setters are a fake insurance policy of isolated change
One supposed advantage of getters and setters is that on the off-chance that the type of a class member needs to change, the change can be limited to inside the class by making the existing getter simply translate from the internal type to the previously-exposed type.
// Car2.java, engine changed to motor
public class Car2 {
private Motor motor;
public Engine getEngine() {
return convertToEngine(motor);
}
public void setEngine(Engine engine) {
this.motor = convertToMotor(engine);
}
}
My question is how often has the working programmer ever had to do that? I don't remember ever doing this in all my years of software. Not once have I been able to take advantage of the fake insurance policy that getters and setters provide.
Also, this argument becomes an entirely moot point if the engine had never been exposed to begin with (let's say it was kept private or package-private). Just expose behavior, rather than state, and you never need to worry about flexibility in changing implementation.
This realization that the private member should not have been exposed triggers another realization that this argument is tautological. Getters and setters expose the private member, and rest the case for their existence on the fact that the private member is exposed.
Getters and setters expose implementation details
Let's say I give you only the following API to my Car object:
_________________________________
| Car |
|---------------------------------|
| + getGasAmount(): Liters |
| + setGasAmount(liters: Liters) |
|_________________________________|
If you assume that this is a gas-powered car that internally tracks gasoline in liters, then you are going to be right 99.999% of the time. That's really bad and this is why getters and setters expose implementation / violate encapsulation. Now this code is brittle and hard to change. What if we want a hydrogen-fuelled car? We have to throw out this whole Car class now. It would have been better just to have behavior methods like fillUp(Fuel fuel).
Things like this are the reason why famous libraries have terrible legacy classes. Have you ever noticed how most languages have a Dictionary data structure but it's called Map in Java? Dictionary actually was an interface that was introduced in JDK 1.0, but it had problems and ultimately had to be replaced by Map.
Getters and setters can actually be dangerous
Let me tell you a story about a friend. OK? A friend I said!!!
One day this friend came into work, and found that dozens of well-known web sites in countries around the world all had the header and navigation of the parent corporation's main web site (not their own), and were using British English. The operations team was frantically restarting hundreds of servers around the globe because for the first half-hour or so that these servers ran, things functioned normally. Then (bam!) all of a sudden something would happen that made the whole thing go sideways.
The culprit was a setter method deep in the guts of a shared platform that all these different sites were using. A little piece of code that ran on a schedule happened to be updated recent to this fiasco that changed the underlying value that determined site headers and languages by calling this setter.
If you only have a getter, things can be just as bad. In Java at least, returning a reference type from a getter provides that reference to the caller and now it can be manipulated by the caller in unexpected ways. Let me demonstrate.
public class Debts {
private List<Debt> debts;
public List<Debt> getDebts() {
return debts;
}
}
OK, that seems reasonable. I need to be able to see a person's debts to give them a statement. Huh? What's that you say? You can add debts now? Shit! How did that happen!
Debts scottsDebts = DebtTracker.lookupDebts(scott);
List<Debt> debts = scottsDebts.getDebts();
// add the debt outside scotts debts, outside the debt tracker even
debts.add(new Debt(new BigDecimal(1000000)));
// prints a new entry with one million dollars
DebtTracker.lookupDebts(scott).printReport();
Eek!
One way to guard against this is to return a copy instead. Another way is to have an immutable member. The best way, though, is to not expose the member in any way at all and instead bring the behavior that manipulates the member inside the class. This achieves full isolation of the implementation and creates only one place to change.
When getters make sense
Wait a second! If there are so many disadvantages to accessors and mutators, why ever use them?
I'm convinced that getters and setters which just return a class member almost never make sense. But you might write something close to getter/setter functionality as long as you are actually doing something in that method.
Two examples:
In a setter, before updating the state in this object according to some input, we validate the input. The input validation is additional functionality.
The return type of a getter is an interface. We have therefore decoupled the implementation from the exposed interface.
See, what I'm really advocating for here is a different stance and philosophy towards getters and setters. Rather than say never use accessors and mutators, I want to give you the list of options that I try to exhaust before using one:
My "default" is to start with a private final member, set only by the constructor. No getter or setter!
If another class absolutely needs to see this member, I think about why. I try to see if there is a behavior that I can expose instead, and create a method for that behavior.
If it's absolutely necessary for some reason, then I relax to package-private (in Java) and expose the member only to other classes in the same package, but no further.
OK, what about the data use case? Literally I might need to have an object to pass data across some kind of interface boundary (let's say to a file system, database, web service, or something). I still don't futz around with getters and setters. I create the class with all package-private members, and I think of and use it as just a bag of properties. I try to limit these into their own packages and layers at the boundaries of the application.
I would consider creating both a getter and a setter for the data use case in a public API, like let's say I am writing a library meant to be used as a dependency in a lot of other applications. But I would only consider it after exhausting all of these list items.
Wisdom of the Masters
A short postscript. Obviously, there's debate about getters and setters out there in the world. It's important to know there's clearly a camp of "masters" like Robert C. ("Uncle Bob") Martin who are proponents of avoiding getters and setters. In the book Clean Code, Martin wrote about this in chapter 6:
Beans have private variables manipulated by getters and setters. The quasi-encapsulation of beans seems to make some OO purists feel better but usually provides no other benefit.
Josh Bloch has a nuanced stance in Effective Java, Item 14 that is slightly in favor of getters and setters for public classes, and slightly against for others. He ends up basically saying that what he is really concerned about is mutability, a point I touched on above:
In summary, public classes should never expose mutable fields. It is less harmful, though still questionable, for public classes to expose immutable fields. It is, however, sometimes desirable for package-private or private nested classes to expose fields, whether mutable or immutable.
Further Reading
Here's some helpful thoughts from smarter people than me.
Oldest comments (92)
You made me think!
Great post!
I see you had two different problems here: The important one, you found a getter returning a pointer, what means you have the control over the content. Aditionally, you put the blame on the getters and setters.
The feature I hate most about Java are equals, getters and setters. They are harder to read (car.setEngine(engine) against car.engine=engine) and harder to write, even with IDEs help, because I need to be generating them and maintaining them if I decide to change the variable name.
They require more code, so when I open the class I have to read those methods just to be sure they are not doing anything more than an assignment. This is time.
And all of this, as you said, to override one in a million.
I found a library, lombok (projectlombok.org/features/GetterS...), that helps you to avoid the implementations with annotations. It is true that it still require to use .getFoo and .setFoo, but I avoid generating them from the IDE and maintain them.
Anyways, I miss a feature in Java like Python's properties, that allow you to override just that method in a million:
So, you can change any public variable to a property just when you require to override that method.
Oh sweet mother of mercy, no. I am currently forced to work with code where someone once decided that getters and setters were evil and did everything with public variables. It's... not good. Yes, 95% of the variables work that way (because we have lot of them), but the remaining 5% make more work than it's worth.
Also your example "gasAmount" is completely nonsenical, because the problem exists no matter if you use getters/setters or just a public variable. How does a public variable with the same name change that problem?
Also your debt example is a perfect example why getters are BETTER than exposing the variables. With a getter you can protect your list by returning a copy. With direct member access? Not really.
So, your arguments against getters and setters are not really good. That's unfortunately, because the basic idea is not a bad one:
While getters/setters ARE better than directly exposing the member, we should of course try to prevent BOTH.
Immutable objects, for example, have far less problems than mutable ones - and if all members are final and imutable themselves, they can be exposed as public members directly, because it's read-only. But the problem this solves is not that getters and setters are but, but that mutability is more dangerous than the other way.
I think you're missing a bit of the point there.
Scott wasn't comparing which of the two (getters vs. public variable) is better. What he said is that getters and setters by default (e.g. when automatically generated by IDE, or shown in beginner examples) expose the variable just the same as making it public; however, people seem to appreciate getters more than public vars, even when they're essentially the same.
The point is reinforced in your last sentence that, above all, the problem is mutability.
Returning copies are not getters (or at least not what most think as getters); they're essentially behaviors. You have to do the extra work to make them only as exposed/encapsulated as necessary.
The point I took from this article is to always strive to make your variables private, and that means not even getters and setters, and only expose behaviors around that data. Which I think makes sense, with some caveats that also have been stated in this article. Not necessarily agreeing, but a good food for thought nonetheless.
That's what I said... "the basic idea is not a bad one" - it was just surrounded by bad arguments for the wrong problems. By starting with the public member thing, the whole text got the wrong spin from the start and the following bad argument didn't help.
Directly addressing mutability, exposing behavior, adding methods that directly work on the members instead of just holding them in a dumb object, etc. is a great idea. But by surrounding it with discussions about public members vs. getters/setters, etc. just dilutes those great points.
I see, and that I can agree with. Hopefully the readers can see beyond it too. Thank you for elaborating!
In the debt example he didn't argue for public variables though, I think you misread that?
He said it wasn't good to expose it at all, but if you do with a getter that returns a copy.
But the better alternative is doing what you want to do with it in the class itself
Yeah, it's too bad many inexperienced developers will not understand that this is just clickbait. Thank you for the great comment. I couldn't do it better.
btw the example of famous person being against setters/getters seem to be about very specific use case.
Well, I used to think that getters and setters are usually useless. Until one day I had to do some computations once a value was set and there I had to re-factor my whole code around the fact that the member was no longer directly accessible but had a getter and a setter.
So I'd say, putting getters and setters is more future proof whilst not being very expensive.
Yet I'd also say that this problem almost only exists in Java because other languages have ways around this (Python <3)
the computations should happen inside your object (in a OO world) you shouldn't be asking other objects for their internals (unless they are simple value objects).
Yup but sometimes you start without needing the computation and then later realize that you need it for a new feature. In that case, it's hard to come back on your decision.
That's why you should program with an OO style and encapsulate. All requires computation.
I was about to mention the same thing. I had the same experience many times.
I've seen quite a couple of articles warning against 'typical' getters and setters usage. What I really appreciate is the nuance you put in explaining when they are appropriate to use.
Thanks for this! You should check out Casey Muratori's epic rant on getters/setters. He's a bit more direct but the message ist mostly the same:
youtube.com/watch?v=_xLgr6Ng4qQ
I love it!
Yeah! 100% agree.
So, if want to change the implementation later of the reference: "public Engine engine"? but 100,000 other classes have directly access, what do you recommend to do?
if before to get a value i need to make some validation, what do you recommend me to do?
Sorry if it wasn't clear. First, I would recommend not making it public to begin with, and definitely not for 100,000 other classes to have access. If you need to make some validation before getting the value, definitely keep the variable private and expose a well-named method instead.
I love C# 😬
Edit: C# Property Design Guidelines
Actually when I just started reading this article. C# is what came to my mind instantly.
I think C# is even worse in this regard.
The argument is that getters/setters and properties VIOLATE ENCAPSULATION by exposing internals as properties rather than mutating through behavior as classic OO would prescribe.
I've seen excessive use of getters/setters/properties in Java, C#, Ruby, and Javascript. Changing the syntax does not help. If anything it aggravates the problem by telling people that its ok to set attributes willy nilly and separate the logic that dictates the behavior from the object.
I've seen huge problems in Ruby where people create "models" and then set all the attributes but forget to encapsulate the "rules" and behaviors around those changes. So later it becomes a huge mess to figure out how the class actually works.
If your "objects" are just a bunch of attributes with just getters and setters then they aren't really objects. They're structs (as in C structs) or records (as in Pascal). And you aren't really doing OO. You're using an OO language to do procedural programming.
Its why Python don't have keywords like public or private.
At the begining I was ..."I hope he doesn't suggest to expose the variables", then
I agree that in most of the cases no variables should be exposes, in any matter, it is a statement of: "please side effects and bug, come and get me". In some env it makes sense because 99% of the time is about public vars (for example in Unity3d you have components, and you need to expose parameters of all sort so the game devs can visually instatiante and configure the instances, and ofc this leads to other big issues, beside the ones mentioned here).
I agree. I think that you should decide the method of your class on the basis of what you expect that class to do (in an abstract sense), rather than on the basis of its internal structure. My personal experience (but YMMV) is that most of the cases (note: not all) where you are tempted to use a setter can be solved with suitable constructor to set the attribute once for all when the object comes to life.
Usually my approach (I use Ada, not Java, so the jargon will be slightly different) is to initially define the internal structure of the object as a "null record," with no field at all. In the spec file then I write the methods I think I need, possibly trying to compile the rest of the code against that spec. Of course, I am not going to obtain an executable, but I can check if the API of my object is complete. Successively, I begin working on the implementation (and this includes the fields of the class).
Usually with this approach any getters/setters you end up with make sense since they do not start from the implementation, but from an abstract view of the object and they are getters/setters "by accident," just because their implementation boils down to reading/writing a field. The resulting code will naturally pass the "acid test" named in one of the linked article why getters and setters method are evil
Hi,
I don't agree with expressing that package private attributes are a better approach than getters and setters.
Suppose you have the case of a bag of properties as you stated in the data use case. A bag of properties may be abstracted as a case of class with state but not behaviour.
Now, let's suppose that, in any time in the future, you have to add behaviour to one of the attributes.
If you implemented the getters and setters, then you put that behaviour in the corresponding methods and you are done.
If you didn't implement those getters and setters, you have several choices to make:
1) Put the new code without being coherent with the rest of the class
2) Refactor all the class to make it coherent and then include the new behaviour
3) Put the behaviour in the calling classes
Maybe I'm missing some point, but I don't see the advantages of the second option.
I didn't understand your option 1, but I would definitely not do option 3. If I understand right, option 3 would mean you go and duplicate the new behavior at every reference to the member. Option 2, then, is way better than 3 in my mind.
For option 2, since it is package private, I would expect the number of references to that field to be fairly limited. How large is a package usually? I'm sure it varies depending on context, but generally its manageable. I personally wouldn't expect more than a few places inside a package where there's a direct reference to a given field. Maybe I make smaller packages than others? With today's refactoring tools, my experience is that its quite painless to switch to a method call rather than a direct member access if that were necessary.
The other thing I would point out is that usually, in the case you mention, you would have to change the method name if you wanted to adhere to the principle of least surprise.
Let's say for a user object, you did start with a getter to reference the user's name, and there were now quite a few references to "user.getFirstName()". Now let's say you got a new requirement to return the user's nickname, if available, rather than their legal first name. Well, if you update the getFirstName() method to do that, yes, you don't have to change code everywhere, but it would kind of suck since it is unexpected for getFirstName to sometimes return the nickname and sometimes return the legal first name.
If we were following good coding practices, what we actually need now is to change all those references from "getFirstName()" to some new method (like "getPreferredName()") instead, which, once again, is no different than if we had used a direct reference to begin with.
Hi Scott, thanks for your answer.
I intended to leave the option 2 as the "more adequate" because the other two are very, very smelly.
Option 2 is more addequate but I wasn't thinking in a case like the one you describe. That case is not a change of behaviour in the getter, but a change in the behaviour of the class. I was thinking in some other cases:
1) Let's say you start with a setter that only does this:
public void setLastName(String ln) {
this.lastName = ln;
}
But in the future, you want to keep track of the modified fields after last read of the database, so in the next update you can only set those fields.
So you add an array of "touched" fields and say, "ok, so when every field is touched by a setter I will add it to the array". Like this:
public void setLastName(String ln) {
this.setTouchedField("lastName");
this.lastName = ln;
}
If you didn't had the setter in the first place, then it would be a pain in the ass adding this behaviour.
2) Let's say you have an object composed of some other objects. Your constructor creates an instance of every each object. But at some point you realize that you want to initialize some of the objects not on parent constructor, but when it's needed. So you could do something like this:
public CustomObject getInstanceOfCustomObject() {
if (this.instanceOfCustomObject == null) {
this.instanceOfCustomObject = new CustomObject();
}
return(this.instanceOfCustomObject);
}
These are some examples I had in mind when I said that it is at some point good to have those getters and setters already coded, even if they are initially totally dumb implementations.
Saludos,
Thanks for the thoughtful questions Daniel! In both cases, I think its still worth asking if there's not some behavior that can be exposed publicly instead of just exposing data. It is easy (because there's so many programmers doing it) to think in terms of data classes being passed around to behavior classes. This is not object-oriented, though. It's a near-cousin to global variables and procedural code operating on those variables. The paper Jason C. McDonald posted in the comments here is a pretty good discussion of these issues.
Unfortunately, both of the examples you gave are going to be prone to concurrency issues that are more serious than the discussion of whether they use "get" style methods. For the second example, try switching to the Initialization-on-demand holder instead. As far as the fact that the method is named "getInstance()" I would not put this in the same category as the getters/setters described in this article. This is a static factory method.
For your example 1, I feel that using the decorator pattern to track the changes in a separate class might be a better approach since it would be a cleaner separation of responsibilities, and you could dynamically add the change-tracking capability only where its needed. OTOH, you don't want to make a whole separate class if its not needed. If you really need to be able to track changes everywhere, then keep it as you showed (but think about concurrency!).
In that case, it would still have a "getXXXX" method then, and I would not be opposed to that. That's why I wrote:
Hi Scott,
Thanks for your answer. Now I understand a little more about the concept you are communicating.
Saludos,
tl;dr: Getters/Setters aren't evil. They are a tool.
Good lord, that's an awful title. People are going to only read that and think" I shouldn't use getters/setters". Please don't follow that advice. I would argue that in Java, NEVER, NEVER, NEVER access a public field of an instance. Always go through an accessor method. Accessors/mutators also can be defined in interfaces, allowing your software to enforce code contracts.
So yes, if you don't need to expose a field, don't. If you can make field immutable, that's great!! But still use an accessor to get it.
Very good write up. I had an undefinable suspicion about getters and setters, but you've helped cement it for me. Thanks for the much-needed kick in the teeth for us OOP purists! :)
I'd like to add to this, if you are only using a class/object for storing data, you've got bigger design issues than just getters and setters. Read Pseudo-Classes and Quasi-Classes
Confuse Object-Oriented Programming by Conrad Weisert.
Thanks for this paper Jason! It is great!
Getters/setters are not OO constructs.. they are inherited from "construction by parts" popularized by languages such as VB and PowerBuilder.
You'll find no such horrors in the old smalltalk codebase (though VisualAge for Smalltalk did have properties because it was a construction by parts system).
So if you were an OO "purist" you would have long eschewed getters and setters.
Fair point. OOP, as Alan Kay (Smalltalk) envisioned it, is nothing like what we have today.
Thanks for the paper by the way.. very interesting.
Some comments may only be visible to logged-in visitors. Sign in to view all comments.