DEV Community

scottshipp
scottshipp

Posted on

Avoid getters and setters whenever possible

Don't generate getters and setters!
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;
}
Enter fullscreen mode Exit fullscreen mode

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;
  }
}
Enter fullscreen mode Exit fullscreen mode

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();
Enter fullscreen mode Exit fullscreen mode

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);
  }
}
Enter fullscreen mode Exit fullscreen mode

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)  |
|_________________________________|
Enter fullscreen mode Exit fullscreen mode

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;
  }
}
Enter fullscreen mode Exit fullscreen mode

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();
Enter fullscreen mode Exit fullscreen mode

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.

Tell, Don't Ask

Why getter and setter methods are evil

Accessors are evil

Top comments (92)

Collapse
 
galdin profile image
Galdin Raphael • Edited

I love C# ๐Ÿ˜ฌ
Edit: C# Property Design Guidelines

Collapse
 
dilantha111 profile image
Dilantha

Actually when I just started reading this article. C# is what came to my mind instantly.

Collapse
 
jodydott profile image
Jody Dott

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.

Thread Thread
 
aminmansuri profile image
hidden_dude

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.

Collapse
 
florianschaetz profile image
(((Florian Schรคtz)))

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.

Collapse
 
bobbypriambodo profile image
Bobby Priambodo • Edited

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.

Collapse
 
florianschaetz profile image
(((Florian Schรคtz)))

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.

Thread Thread
 
bobbypriambodo profile image
Bobby Priambodo

I see, and that I can agree with. Hopefully the readers can see beyond it too. Thank you for elaborating!

Collapse
 
hrmny profile image
Leah

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

Collapse
 
akostadinov profile image
Aleksandar Kostadinov

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.

Collapse
 
xowap profile image
Rรฉmy ๐Ÿค–

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)

Collapse
 
jodydott profile image
Jody Dott

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

Collapse
 
xowap profile image
Rรฉmy ๐Ÿค–

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.

Thread Thread
 
jodydott profile image
Jody Dott

That's why you should program with an OO style and encapsulate. All requires computation.

Collapse
 
rafiq74 profile image
rafiq74

I was about to mention the same thing. I had the same experience many times.

Collapse
 
dimpiax profile image
Dmytro Pylypenko

You have just shown bad design. Reason not in getter/setter, but in approach.
If you open properties to public you must have reason why. But not like exception with debt example.

If replace getter/setter theme with singleton or composite โ€“ and write in the same manner, the result that all is bad, will be the same.

Main point: use features properly. We have instruments in our belts, and to hammer nails with a saw is not the best choice.

Collapse
 
scottshipp profile image
scottshipp

Hi Pilipenko, my goal with this article was to show bad design. All of the code examples here are things I would not do, including the debt example. I tried to give guidelines for what I would do in the section at the end, but I probably could follow up with code examples of what I think people should do.

Collapse
 
dimpiax profile image
Dmytro Pylypenko

But your topic name is "Avoid getters and setters whenever possible", not "Bad design in getter/setter". This article just discredits specific area of programming, but not educational.

We have a lot of useless articles like "Don't use OOP", "Don't use functional programming", "Why declarative worse that imperative" and so on. But whole idea in right usage, and neither getter/setter neither functional or AOP are not bad.

Thread Thread
 
xtofl profile image
xtofl

I must say you have a point: we lack a vast body of "how to do X without violating design flaw Q".

This year I learned about Domain Driven Design, and the Onion Architecture, where they design apis around use cases instead of data. Maybe there is some source of positive guidelines we can tap.

Collapse
 
codemouse92 profile image
Jason C. McDonald • Edited

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.

Collapse
 
aminmansuri profile image
hidden_dude

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.

Collapse
 
codemouse92 profile image
Jason C. McDonald

Fair point. OOP, as Alan Kay (Smalltalk) envisioned it, is nothing like what we have today.

Thread Thread
 
aminmansuri profile image
hidden_dude

Thanks for the paper by the way.. very interesting.

Collapse
 
scottshipp profile image
scottshipp

Thanks for this paper Jason! It is great!

Collapse
 
kallaste profile image
Tatiana McGarry • Edited

Getters and setters are an integral part of encapsulation in OOP. Yes, things can go wrong when you abuse a setter, but unfortunately, there is no such thing as "idiot proof" code. It used to be that programmers themselves were expected to have brains.

Everywhere I look there are new "suggested practices" popping up that seem to want to guard against someone else's poor programming messing up your code in the future, when unfortunately, that simply cannot be done. The suggestion to "avoid getters and setters whenever possible" is just another example. But inexperienced developers read an article like this one and just jump on the bandwagon. How about we simply omit them when it makes sense to do so?

Just program responsibly and think about your use cases when you need something to be inaccessible and/or immutable, and stop handicapping yourself trying to guard against any remote possibility of some future developer using poor practices in the future.

Collapse
 
aminmansuri profile image
hidden_dude

They are not part of encapsulation. They are the opposite.

An encapsulated class in "pure OO" does not expose its properties. But many people confuse this because their only exposure to "OO" has been through Java and C# (and their successors) and have had little exposure to a real OO system like Smalltalk.

Collapse
 
kallaste profile image
Tatiana McGarry • Edited

Yes, they are a totally legitimate part of encapsulation in the current paradigm of Object Oriented Programming, and will be mentioned as such in current literature about it. That isn't confusion, it is the maturation of programming language concepts. A lot has changed since 40 years ago when Smalltalk was invented. We had no idea what OOP was going to be back then, or in many ways how it was going to be used.

The argument that getters and setters aren't a valid part of encapsulation because Smalltalk did it differently is a little like debating what the World Wide Web is on the basis of how things were done in ARPANET. Quite simply, just not very relevant.

Thread Thread
 
aminmansuri profile image
hidden_dude • Edited

No. Its a misunderstanding about what encapsulation is.

If you have a computation like:

bankAcct.setBalance(bankAcct.getBalance() * bankAcct.getInterestRate())

Then you aren't understanding how OO is supposed to work. The correct encapsulation would be by having behavior based method:

bankAcct.accrueInterest()

Getters and Setters don't come from OO they come from "construction by parts" a style of programming made for automated UIs like in VB. It has its place, its related to OO, but it isn't encapsulation. It's the opposite. Its exposing your attributes to the outside world.

And plenty of OO was written by great programmers like Kent Beck and others that later helped influence how we did it in other languages. But the popularization of many techniques has little to do with good OO, and simply popularization of stuff that certain developers promoted.

Collapse
 
tmfrisinger profile image
Travis Frisinger • Edited

Fantastic stuff! Ultimately, it is the lack of encapsulation and presence of mutability that are the real problems. It is not a style or language issue, rather, it is the lack of awareness from developers to push for better solutions that do not rely on 'cheap and nasty' to do the job.

Collapse
 
elcotu profile image
Daniel Coturel

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.

Collapse
 
scottshipp profile image
scottshipp

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.

Collapse
 
elcotu profile image
Daniel Coturel

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,

Thread Thread
 
scottshipp profile image
scottshipp

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:

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.

Thread Thread
 
elcotu profile image
Daniel Coturel

Hi Scott,
Thanks for your answer. Now I understand a little more about the concept you are communicating.
Saludos,

Collapse
 
wrongabouteverything profile image
wrong-about-everything

Totally agree! Accessors is totally anti-OOP feature. I believe it was Martin Fowler who popularized a reprobation of this concept in his post Anemic Domain Model. But little changed from 2003. A lot of people still are not aware that the whole point of OOP is combining data and behavior. Thus accessors still don't make any sense there.

Generally, the $64k question is how to come up with such an objects that expose only behavior, not data. Well, my take on it that it all starts with the problem space -- your concrete domain at hand. Talk to your business-experts, consider the use of CRC-cards, draw use-case diagrams and find your smart and knowledgeable objects.

Collapse
 
aminmansuri profile image
hidden_dude • Edited

There are some solutions to the getter/setter problem but they aren't great. Like using a visitor pattern to feed data into a view, or using "renderer objects". Part of the reason for getters and setters is the more pressing desire to separate model's and views.

But I agree that from a purist point of view, getters/setters are not really OO and they violate encapsulation.

Collapse
 
pocketstealer profile image
Pocketstealer

Get/Setter.

Why? Because it's easier to say in a set:
If(Value not null){
set value
}
else{
w/e
}

Than to go EVERYWHERE and check if that value is null then use something.

Sure a basic example looks stupid... but you were not taught not touching the flames by being explained what thermodynamics are...

Get/Setter's are better for a good approach and have assurance that the class will work no matter what the values will be given.

PHP is a loosely typed language getter/setter are perfect for this.

Collapse
 
scottshipp profile image
scottshipp

You're concerned about code duplication and that's a good concern. But what if there's a bigger issue here? The fact that this setter can be called "EVERYWHERE", as you say, is a bigger problem to me. Everyone has access to set this value. If just one of them makes a mistake, everyone else using it suffers the downstream effects.

So the real problem here isn't duplicated code, to my mind. It's the fact that there's a variable available everywhere. That sounds like a global variable with some procedural code manipulating its state.

Consider what happens if the field is kept private, and no get/set methods are made public either. Where is everywhere now? Only inside the class itself. It's not a global variable anymore. There aren't a bunch of random classes out there that can go set that value. Another developer can't come in with some new code that calls set on that value incorrectly, and stops several user scenarios from working correctly.

I think we've all worked in scary code like this and we can do better. In my experience, I've found that looking at everywhere the set method is called yields the result that I come out with at most a few different uses, and I can write a method for each use and call the new method from these call sites instead. They'd be behavior methods which keep that variable inside a class or at worst inside a package, and not available everywhere. That way I not only de-duplicate code, but I prevent other bugs that could occur as well. Bugs like the site header one that I mentioned. When no one can call setWhatever(newValue)--indeed they don't even know that the Whatever variable exists--then those kinds of things aren't possible.

Collapse
 
jstormes profile image
James Stormes

In PHP we also use reflection for hydrating objects. If all your objects properties have getters and setters you can hydrate by reflection, otherwise you have to write lots of exchange_array code.

However we can also hydrate on properties directly, but you don't get to do any validation of the data. As you say PHP is loosely typed, so that opens you up to hacking/injection.

What is harder to do is hydrate on both get/set and properties. So you wind up all ways using setters, even if you don't need them all the time.

Collapse
 
pocketstealer profile image
Pocketstealer

I think we are confusing bad practice with the role of getter/setter.

Bad practice is everywhere. But when you are using a getter/setter you expect to give a certain value and get a certain value.

You know you are setting a string. You will get a string. Thats the role of the getter/setter.
You are certain and sure that there will be in no circumstation a null there. Or a number. Always a string.

Using getter/setter for everything is bad as well. Leaving a variable there that is used and can be moddified to contain anything is even worse.

If you use get/set just to "return","$this->alfa = value" that's bad practice.

But a beginner doesn't know when should use it or not. So the one that's easiest to "implement after" it's the cheapest.

Scope of projects change. Agile is here and not always you have time to get all the information/specs or maybe some new information get there with constraints.

Always take the safest/easiest way rather than "smart way but it might blow up later".

Collapse
 
arhanger profile image
Arhanger

Someone has never changed his code. Sorry, but afet this "In both classes, Car1.java and Car2.java, we get essentially the same result" i stopped reading. Public properties are not the same as getters and setters. With getters and setters we have an entry point to our business logic, with public properties we don't have any. After changing hundreds of calls to any property through the whole project you'll understand why. This is the first point.
The second point was mentioned in comments. Calls by reference. When you use public properties you usually don't bother with copying stuff. Why should I? Doing that hundred times. That leads to a couple of painfull bugs. The most common one are date objects being modified by reference. Hours thrown away trying to debug that one. Especially by middle/juniors.
Third, getters and setters can be covered with unit tests. If someone changes something you will see that.
The last and not least. It's a lot harder to control workflow with this approach. Instead of reviewing code design, you are forced to check every property call through the whole project. Not very effective.
It's not bad to try to find something new, but getters and setters are used not by chance. If you want to remove a tool, propose something to cover that gap. Good luck.

Collapse
 
mduran profile image
Miguel Duran

It shows that you stopped reading at that point. His point was not that public attributes are just as good or better than getters and setters, his point was that the internals of a class should be as restricted as possible and having getters and setters violates that.

If you actually read the article, you would see this:


```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.```

Collapse
 
arhanger profile image
Arhanger

My points wasn't on good/bad. He said that both ways give you the same result. And this is wrong. That was what I wrote about.

Second, but not least. Getters and setters can also be restricted, if you need that.

Third, there could be dozens of parameteres. Constructor will become huge and I personally don't like that. For that I'd better use a factory, that will garantee to create a valid object. For any other call there's code review to deny them.

Fourth, there's metaprograming in many languages. That lets you use even private members. So I don't see a reason to be so paranoic about that. But private members could give you headache when writing unit tests or changing class with tens of references to that private member.

To conclude, my main point was that these two methods don't give you the same result. Hope that makes my thougts clearer.

Collapse
 
aminmansuri profile image
hidden_dude

Getters and setters often violate the principle of encapsulation which is to NOT expose internal details of your class.

They aren't really an OO technique but rather a "construction by parts" technique popularized by UI oriented techniques from VB or PowerBuilder. They are designed so that automated UI tools can get and set attributes of a UI component like color, size, etc.. It was later used for "JavaBeans" (that also started in the UI) and then was used by frameworks like Hibernate.

But to suggest this is good OO is just because many people never saw real OO from the Smalltalk days.
If you have something like a Car with a getX() and getY() position then you set the x and y position you are overriding whatever animation sequence that car should do when you "drive to" a position. A Car should really drive to positions not simply jump across the screen.

Methods should encapsulate BEHAVIOR not simple data mutation. That being said, there is room for objects such as value objects to pass around.

Another phenomena that is becoming apparent is that in backend Web programming many of the techniques used today are not really where OO shines. OO shines more in GUI and front end designs where you have inheritance and composition between components.

So I have to agree with the article.

Collapse
 
smontiel profile image
Salvador Montiel • Edited

You made me think!

Great post!

Collapse
 
marlysson profile image
Marlysson Silva

Awesome approach and good arguments.

I don't know if you knows the "jsf" ( web programming in java ) , that some specific classes that requires the getters and setters ( convention over configuration ) to their internal implementations uses all structure of the application that use some implementation.

Do you know some advice about it? Or isn't related to the post and there aren't to do?

Collapse
 
scottshipp profile image
scottshipp

I think I should have tackled this in the article. You are going to be on the hook to use getters and setters for libraries (like JSF) that reflect on them. Can't really avoid that.

But you can avoid a dangerous anti-pattern (in my opinion), which is then passing such objects around inside the application.

Usually, Java libraries that use reflection on getters and setters are doing it to bind data to/from these objects. There is usually a specific intent behind this, such as to create a POJO from incoming data, such as from the JSON body of an http request.

I believe that this type of automatic object creation should be limited to a single place at the edge of the application. If the usual pattern is followed of turning around and passing the resulting object around inside the app, it creates a situation where changes in the JSON body (or whatever it is), makes you have to change all the code paths that the object travels across.

So its best if you can isolate and decouple these auto-generated objects from your application. Build walls around them and keep them at the edges.

Collapse
 
aminmansuri profile image
hidden_dude

There are ways around getters/setters/properties but they aren't very simple. Like using a visitor pattern or renderers. They definitely aren't popular and would confuse the average programmer.

For some things getters/setters, while not real OO, are a good solution. For example the way Hibernate handles DB properties, or ActiveRecord. As long as people remember to wrap these with real objects later that expose behavior instead of attibutes.

Collapse
 
bgadrian profile image
Adrian B.G.

At the begining I was ..."I hope he doesn't suggest to expose the variables", then

My "default" is to start with a private final member, set only by the constructor. No getter or setter!
Ok, we're all good, move on.

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

Some comments may only be visible to logged-in visitors. Sign in to view all comments.