53

Allen Holub wrote the following,

You can't have a program without some coupling. Nonetheless, you can minimize coupling considerably by slavishly following OO (object-oriented) precepts (the most important is that the implementation of an object should be completely hidden from the objects that use it). For example, an object's instance variables (member fields that aren't constants), should always be private. Period. No exceptions. Ever. I mean it. (You can occasionally use protected methods effectively, but protected instance variables are an abomination.)

Which sounds reasonable, but he then goes on to say,

You should never use get/set functions for the same reason—they're just overly complicated ways to make a field public (though access functions that return full-blown objects rather than a basic-type value are reasonable in situations where the returned object's class is a key abstraction in the design).

Which, frankly, just sounds insane to me.

I understand the principle of information hiding, but without accessors and mutators you couldn't use Java beans at all. I don't know how you would follow a MVC design without accessors in the model, since the model can not be responsible for rendering the view.

However, I am a younger programmer and I learn more about Object Oriented Design everyday. Perhaps someone with more experience can weigh in on this issue.

Allen Holub's articles for reference


Related Questions:

5
  • 1
    I'm pretty sure we're done this a couple of times already, but I can't find any instances... Jun 15, 2009 at 13:59
  • dmckee, if you find any let me know. The search engine sometimes lets you down, especially for more abstract ideas like the one presented here. Jun 15, 2009 at 14:02
  • Google came through with a few that are close... Jun 15, 2009 at 14:10
  • 2
    Don't know why this is marked as a duplicate when the other question was asked months after this Aug 12, 2016 at 1:06
  • Holub is not alone. He's joined by Alan Kay, the inventor or Object Oriented programming. In this video Alan Kay explains why he thinks it's a bad idea. youtube.com/watch?v=YyIQKBzIuBY&t=1152s Dec 6, 2022 at 17:36

13 Answers 13

40

I don't have a problem with Holub telling you that you should generally avoid altering the state of an object but instead resort to integrated methods (execution of behaviors) to achieve this end. As Corletk points out, there is wisdom in thinking long and hard about the highest level of abstraction and not just programming thoughtlessly with getters/setters that just let you do an end-run around encapsulation.

However, I have a great deal of trouble with anyone who tells you that you should "never" use setters or should "never" access primitive types. Indeed, the effort required to maintain this level of purity in all cases can and will end up causing more complexity in your code than using appropriately implemented properties. You just have to have enough sense to know when you are skirting the rules for short-term gain at the expense of long-term pain.

Holub doesn't trust you to know the difference. I think that knowing the difference is what makes you a professional.

1
  • 7
    I like that point: A professional knows when (and how) to apply techniques effectively... a hacker "just gets it working".
    – corlettk
    Jun 19, 2009 at 10:46
34

Read through that article carefully. Holub is pushing the point that getters and setters are an evil "default antipattern", a bad habit that we slip into when designing a system; because we can.

The thought process should be along the lines; What does this object do? What are its responsibilities? What are its behaviours? What does it know? Thinking long and hard on these questions leads you naturally towards designing classes which expose the highest-level interface possible.

A car is a good example. It exposes a well-defined, standardised high-level interface. I don't concern myself with setSpeed(60)... is that MPH or km/h? I just accelerate, cruise, decelerate. I don't have to think about the details in setSteeringWheelAngle(getSteeringWheelAngle()+Math.rad(-1.5)), I just turn(-1.5), and the details are taken care of under the hood.

It boils down to "You can and should figure out what every class will be used for, what it does, what it represents, and expose the highest level interface possible which fulfills those requirements. Getters and setters are usually a cop-out, when the programmer is just to lazy to do the analysis required to determine exactly what each class is and is-not, and so we go down the path of "it can do anything". Getters and setters are evil!

Sometimes the actual requirements for a class are unknowable ahead of time. That's cool, just cop-out and use getter/setter antipattern for now, but when you do know, through experience, what the class is being used for, you'll probably want to comeback and cleanup the dirty low level interface. Refactoring based on "stuff you wish you knew when you write the sucker in the first place" is par for the course. You don't have to know everything in order to make a start, it's just that the more you do know, the less rework is likely to be required upon the way.

That's the mentality he's promoting. Getters and setters are an easy trap to fall into.

Yes, beans basically require getters and setters, but to me a bean is a special case. Beans represent nouns, things, tangible identifiable (if not physical) objects. Not a lot of objects actually have automatic behaviours; most times things are manipulated by external forces, including humans, to make them productive things.

daisy.setColor(Color.PINK) makes perfect sense. What else can you do? Maybe a Vulcan mind-meld, to make the flower want to be pink? Hmmm?

Getters and setters have their ?evil? place. It's just, like all really good OO things, we tend to overuse them, because they are safe and familiar, not to mention simple, and therefore it might be better if noobs didn't see or hear about them, at least until they'd mastered the mind-meld thing.

6
  • 3
    Yeah I think the initial statement was sensationalism. An article using a correct level of abstraction, which most people agree with, probably garners less hits then making a incendiary statement. Jun 15, 2009 at 14:51
  • 1
    With my car, I tend to have the speedometer set to mph, select a speed with the gas pedal, and confirm it with the cruise control. It's about as direct an implementation as you're going to get for setSpeed(60) without a keypad. Jun 15, 2009 at 14:59
  • 5
    not knowing if setSpeed(60) is in mph or kmh will put you in jail. Poor example.
    – Jason
    Jun 19, 2009 at 6:19
  • 6
    "But officer I was programming to an unclear interface!" Jun 19, 2009 at 11:04
  • OK, the mph/kmph thing was a poor "real world" example, but it still serves to illustrate the point that the interface should be in at the high-as-possible level... and BTW, I'm thinking MPH/KMPH should be localised.
    – corlettk
    Mar 2, 2010 at 7:20
31

I think what Allen Holub tried to say, rephrased in this article, is the following.

Getters and setters can be useful for variables that you specifically want to encapsulate, but you don't have to use them for all variables. In fact, using them for all variables is nasty code smell.

The trouble programmers have, and Allen Holub was right in pointing it out, is that they sometimes use getters/setters for all variables. And the purpose of encapsulation is lost.

5
  • 16
    Using Getter/Setter methods up front ensure that they are used throughout the software's life. If you mix them up front and then find out later down the road that Encapsulation is needed...you need to re-write any code that was referencing that public value. Jun 15, 2009 at 13:49
  • 3
    +1 Having a container class with getters/setters is reasonable. Having just any class with lots of getters/setters can be a code smell.
    – sharptooth
    Jun 15, 2009 at 13:50
  • Exactly right. Used properly, getters/setters encapsulate. You don't have to know the internals of the getter/setter, or its operations on object state, in order to use it. Jun 15, 2009 at 13:52
  • 3
    @Justin Niessner, he does not say to replace accessors with public fields. In fact, the only thing he's clear about is that accessors return type should be interfaces. Jun 15, 2009 at 14:29
  • +1 to @Justin. Even if the class is all accessors, most likely the IDE is doing it for you anyway, and your access is consistent.
    – orbfish
    Nov 6, 2011 at 2:58
10

(note I'm coming at this from a .NET "property" angle)

Well, simply - I don't agree with him; he makes a big fuss about the return type of properties being a bad thing because it can break your calling code - but exactly the same argument would apply to method arguments. And if you can't use methods either?

OK, method arguments could be changed as widening conversions, but.... just why... Also, note that in C# the var keyword could mitigate a lot of this perceived pain.

Accessors are not an implementation detail; they are the public API / contract. Yup, if you break the contracft you have trouble. When did that become a surprise? Likewise, it is not uncommon for accessors to be non-trivial - i.e. they do more than just wrap fields; they perform calculations, logic checks, notifications, etc. And they allow interface based abstractions of state. Oh, and polymorphism - etc.

Re the verbose nature of accessors (p3?4?) - in C#: public int Foo {get; private set;} - job done.

Ultimately, all of code is a means to express our intent to the compiler. Properties let me do that in a type-safe, contract-based, verifiable, extensible, polymorphic way - thanks. Why do I need to "fix" this?

1
  • Oh how I wish Java had C# style properties support. Well there is always Groovy. Jun 16, 2009 at 12:01
9

Getters and setters are used as little more than a mask to make a private variable public.

There's no point repeating what Holub said already but the crux of it is that classes should represent behaviour and not just state.

8

Some opposing views are in italics:

Though getIdentity starts with "get," it's not an accessor because it doesn't just return a field. It returns a complex object that has reasonable behavior

Oh but wait... then it's okay to use accessors as long as you return objects instead of primitive types? Now that's a different story, but it's just as dumb to me. Sometimes you need an object, sometimes you need a primitive type.

Also, I notice that Allen has radically softened his position since his previous column on the same topic, where the mantra "Never use accessors" didn't suffer one single exception. Maybe he realized after a few year that accessors do serve a purpose after all...

Bear in mind that I haven't actually put any UI code into the business logic. I've written the UI layer in terms of AWT (Abstract Window Toolkit) or Swing, which are both abstraction layers.

Good one. What if you are writing your application on SWT? How "abstract" is really AWT in that case? Just face it: this advice simply leads you to write UI code in your business logic. What a great principle. After all, it's only been like at least ten years since we've identified this practice as one of the worst design decisions you can make in a project.

My problem is as a novice programmer is sometimes stumbling onto articles on the internet and give them more credence then I should. Perhaps this is one of those cases.

1
  • 1
    I think one of the biggest things people need to learn about programming it to take all of this "advice" with a grain of salt. And use the brain you have... (That’s why it's there after all) Jun 15, 2009 at 14:20
5

When ideas like these are presented to me, I like to take a look at libraries and frameworks I use and which I like using.

For example, although some will disagree, I like the Java Standard API. I also like the Spring Framework. Looking at the classes in these libraries, you will notice that very rarely there are setters and getters which are there just to expose some internal variable. There are methods named getX, but that does not mean it is a getter in the conventional sense.

So, I think he has a point, and it is this: every time you press choose "Generate getters/setters" in Eclipse (or your IDE of choice), you should take a step back and wonder what you are doing. Is it really appropriate to expose this internal representation, or did I mess up my design at some step?

1
  • Well said... in fact I THINK that's the crux of what I was trying to say... just your version is far more succinct, and therefore accesible in "the critical moment" when you're making such choices. Two thumbs up from me.
    – corlettk
    Oct 8, 2012 at 7:24
2

I don't believe he's saying never use get/set, but rather that using get/set for a field is no better than just making the field public (e.g. public string Name vs. public string Name {get; set; }).

If get/set is used it limits the information hiding of OO which can potentially lock you into a bad interface.

In the above example, Name is a string; what if we want to change the design later to add multiple Names? The interface exposed only a single string so we can’t add more without breaking existing implementation.

However, if instead of using get/set you initially had a method such as Add(string name), internally you could process name singularly or add to a list or what not and externally call the Add method as many times as you want to add more Names.

The OO goal is to design with a level of abstraction; don’t expose more detail than you absolutely have to.

Chances are if you’ve just wrapped a primitive type with a get/set you’ve broken this tenet.

Of course, this is if you believe in the OO goals; I find that most don't, not really, they just use Objects as a convienient way to group functional code.

2
  • Well in that particular article he is literally saying "never use get/set functions". I believe he back pedaled on it later. However it means he made the initial statement either a) without proper thought b) to be inflammatory. Either is pretty damning to his creditability. Jun 15, 2009 at 14:37
  • 1
    Getters and setters are considerably better than having public fields. They give you a last chance to maintain class invariants and to provide hooks. Jun 15, 2009 at 14:46
2

Public variables make sense when the class is nothing more than a bundle of data with no real coherency, or when it's really, really elementary (such as a point class). In general, if there's any variable in a class that you think probably shouldn't be public, that means that the class has some coherence, and variables have a certain relation that should be maintained, so all variables should be private.

Getters and setters make sense when they reflect some sort of coherent idea. In a polygon class, for example, the x and y coordinates of given vertices have a meaning outside the class boundary. It probably makes sense to have getters, and it likely makes sense to have setters. In a bank account class, the balance is probably stored as a private variable, and almost certainly should have a getter. If it has a setter, it needs to have logging built in to preserve auditability.

There are some advantages of getters and setters over public variables. They provide some separation of interface and implementation. Just because a point has a .getX() function doesn't mean there has to be an x, since .getX() and .setX() can be made to work just fine with radial coordinates. Another is that it's possible to maintain class invariants, by doing whatever's necessary to keep the class consistent within the setter. Another is that it's possible to have functionality that triggers on a set, like the logging for the bank account balance.

However, for more abstract classes, the member variables lose individual significance, and only make sense in context. You don't need to know all the internal variables of a C++ stream class, for example. You need to know how to get elements in and out, and how to perform various other actions. If you counted on the exact internal structure, you'd be bogged down in detail that could arbitrarily vary between compilers or versions.

So, I'd say to use private variables almost exclusively, getters and setters where they have a real meaning in object behavior, and not otherwise.

Just because getters and setters are frequently overused doesn't mean they're useless.

5
  • Ummm, dude... A bank accounts balance is derivable; and therefore should not be stored anywhere, except in a local variable diring re-calculation ;-) Cheers. Keith.
    – corlettk
    Jun 19, 2009 at 10:49
  • Derivable in what way? I'd rather store it than start with zero and add up all the deposits, withdrawals, interest, and adjustments. I'm genuinely curious: how would you derive it and from what? Jun 19, 2009 at 14:31
  • Dave, I worked on a BIG government accounting system for 7 years. Yep, that's exactly how an accounting system works. The only persistent-balances are stored in "temporary" tables, used in processing, reporting, and so on... The single point-of-truth for the balance of an account IS the source-transaction-records and the business-algorithm used to calculate it. Permanently storing derivable factiods is evil, because it leads directly to unproven, unreproducable results! Trust me on this.
    – corlettk
    Jun 20, 2009 at 3:14
  • 1
    So you start at some audited point and run all transactions to get the current balance? (I assume that starting at day 0 and running all transactions would get cumbersome after a few decades.) Interesting. Jun 22, 2009 at 14:14
  • Cumbersome, yes... but then I suppose I was working on LAND systems, and land "keeps" pretty well, and therefore so must the transactions on that land... hence the "do EVERYthing from source" appoach... but things would get VERY interesting IF we ever needed to (like maybe an overnight tenfold increase in the cost of diskspace or CPU) archive the transactions and work from "intermediate balances"... Stock exchange transactions (ergo billions of) would be a completely different story, I guess.
    – corlettk
    Oct 8, 2012 at 7:02
2

The trouble with getters/setters is they try to fake encapsulation but they actually break it by exposing their internals. Secondly they are trying to do two separate things - providing access to and controlling their state - and end up doing neither very well.

It breaks encapsulation because when you call a get/set method you first need to know the name (or have a good idea) of the field you want to change, and second you have to know it's type eg. you couldn't call

setPositionX("some string");

If you know the name and type of the field, and the setter is public, then anyone can call the method as if it were a public field anyway, it's just a more complicated way of doing it, so why not just simplify it and make it a public field in the first place.

By allowing access to it's state but trying to control it at the same time, a get/set method just confuses things and ends up either being useless boiler-plate, or misleading, by not actually doing what it says it does by having side-effects the user might not expect. If error checking is needed, it could be called something like

public void tryPositionX(int x) throws InvalidParameterException{
    if (x >= 0)
        this.x = x;
    else
        throw new InvalidParameterException("Holy Negatives Batman!");
}

or if additional code is needed it could be called a more accurate name based on what the whole method does eg.

tryPositionXAndLog(int x) throws InvalidParameterException{
    tryPositionX(x);
    numChanges++;
}

IMHO needing getters/setters to make something work is often a symptom of bad design. Make use of the "tell, don't ask" principle, or re-think why an object needs to send it's state data in the first place. Expose methods that change an object's behaviour instead of it's state. Benefits of that include easier maintenance and increased extensibility.

You mention MVC too and say a model can't be responsible for it's view, for that case Allen Holub gives an example of making an abstraction layer by having a "give-me-a-JComponent-that-represents-your-identity class" which he says would "isolate the way identities are represented from the rest of the system." I'm not experienced enough to comment on whether that would work or not but on the surface it sounds a decent idea.

1

Public getters/setters are bad if they provide access to implementation details. Yet, it is reasonable to provide access to object's properties and use getters/setters for this. For example, if Car has the color property, it's acceptable to let clients "observe" it using a getter. If some client needs the ability to recolor a car, the class can provide a setter ('recolor' is more clear name though). It is important to do not let clients know how properties are stored in objects, how they are maintained, and so on.

0

Ummmm...has he never heard of the concept of Encapsulation. Getter and Setter methods are put in place to control access to a Class' members. By making all fields publicly visible...anybody could write whatever values they wanted to them thereby completely invalidating the entire object.

Just in case anybody is a little fuzzy on the concept of Encapsulation, read up on it here:

Encapsulation (Computer Science)

...and if they're really evil, would .NET build the Property concept into the language? (Getter and Setter methods that just look a little prettier)

EDIT

The article does mention Encapsulation:

"Getters and setters can be useful for variables that you specifically want to encapsulate, but you don't have to use them for all variables. In fact, using them for all variables is nasty code smell."

Using this method will lead to extremely hard to maintain code in the long run. If you find out half way through a project that spans years that a field needs to be Encapsulated, you're going to have to update EVERY REFERENCE of that field everywhere in your software to get the benefit. Sounds a lot smarter to use proper Encapsulation up front and safe yourself the headache later.

3
  • 6
    I think you've missed his point - in most (if not all) classes, there are some variables that should not be touched by client code. For those variables, you don't add getter/setter methods. Jun 15, 2009 at 13:58
  • 2
    That, I would agree with... but that is not what he said. He was saying it is better to have some data fields as just public fields instead of public properties... might look cleaner in Java but in .Net properties are first class and are (in my opinion) preferred. I don’t event really like protected fields. I would prefer protected properties for clean inheritance. Jun 15, 2009 at 14:17
  • I definitely agree with Matthew. Getters and Setters being evil has nothing to do with an architect deciding which elements of his class to expose to the public or hide from them. That's another discussion entirely. Jun 15, 2009 at 14:22
0

I think that getters and setters should only be used for variables which one needs to access or change outside a class. That being said, I don't believe variables should be public unless they're static. This is because making variables public which aren't static can lead to them being changed undesirably. Let's say you have a developer who is carelessly using public variables. He then accesses a variable from another class and without meaning to, changes it. Now he has an error in his software as a result of this mishap. That's why I believe in the proper use of getters and setters, but you don't need them for every private or protected variable.

1
  • " public which aren't static can lead to them being changed undesirably"... Umm, are you confusing static with readonly/const (depending on which language you're playing in at the time).
    – corlettk
    Oct 8, 2012 at 7:04

Not the answer you're looking for? Browse other questions tagged or ask your own question.