getters/setters are evil?

This is in response to Holub's article at JavaWorld:
http://www.javaworld.com/javaworld/jw-09-2003/jw-0905-toolbox.html?
He argues that getters/setters (accessors) are overused, and since I only have a few years of exposure to Java (most of it in the university and very little in the "real world"), I'm wondering if some more experienced OO-minded Java folks could help me understand this (if you agree). He also argues that you don't really need to have getters/setters with beans and that Sun failed to see the bad turn Java coders would take in overusing getters/setters with beans.
All I know is that it was emphasized very strongly with me to make fields private and always provide public getters/setters for them. I guess maybe they were trying to emphasize the OO aspects of asking other objects to do something for me rather than seeing those objects as struct-like data warehouses that I could just pull information out of directly. Would you say this is correct? Thus, put an end to most getters/setters for members and instead focus on having objects do something for me and give me the fruits of their labor?

There are people who make a living out of writing controversial statements. They will take a commonly held viewpoint and then purposely take the opposite view, simply so that they can drum up interest in their writing.
While I'm stopping short of saying that's what Allen Holub is doing, I will emphatically disagree with his statement that somehow setters and getters are non-OO. He equates getters and setters with public fields, somehow missing the point that getters and setters specifically avoid public fields. And I always have distrust for someone who calls a programming technique "evil". The title of the article in question was "Why getters and setters are evil" - not too National Enquirer, eh?
Anyway, Mr. Golub evidently never worked in a language with global naming conventions, like RPG or old BASIC dialects. Had he done so, he would understand the horrors of public variables, as opposed to those with controlled access via accessors.
He has a mild point in that not every value that needs a getter also needs a setter, and vice versa. I tend to agree with that; blind creation of getters and setters for every field in a class is overkill. But to state that accessors are non-OO displays a unique view of object orientation.
My conclusion? The man has a point that some classes probably have too many getters and setters, and IDEs tend to make it very easy to create those unnecessary accessors. However, his contention that accessors are non-OO is pure rubbish, and detracts from the rest of his article. And no matter what, the idea of making fields public in any except the most trivial of data transport beans is a dangerous one. I only hope nobody that listens to him ever codes in one of my projects. And if you read the comments, the majority of the readers agree with me.
Bottom line is that common sense still prevails: all fields should be private, but only write the setters and getters you need.
Joe

The article has been discussed at TheServerSide. As already pointed out, don't take the article's subject line as a fact before reading the responses and analyzing yourself whether the case is solid.

When a class contains a getter and setter for something it shouldn't, those are evil.
When another class misses them for something that should be exposed, that's evil too.
Both are examples of poor design (or lacking implementation) and not of "getters and setters are evil" or "getters and setters are required".
For example, if you encapsulate a List in a class, it often makes more sense to provide getters and setters for entries from that List than for the List as an entity (since the List is merely a container for the actual data rather than the data itself).

I feel that in the get method we need to return the copy of the property by calling the clone method.
What do you guys feel?

Originally posted by Joe Pluta:
And no matter what, the idea of making fields public in any except the most trivial of data transport beans is a dangerous one.

But he isn't proposing to make the fields public. In fact he writes:

Getter and setter methods (also known as accessors) are dangerous for the same reason that public fields are dangerous

Instead, he proposes to fully hide the data and expose behaviour instead. And I think he is right in doing so.
See http://c2.com/cgi/wiki?AccessorsAreEvil for more on the topic.

Originally posted by S. C. Huey:
Thus, put an end to most getters/setters for members and instead focus on having objects do something for me and give me the fruits of their labor?

Exactly!

Originally posted by S. C. Huey:
Thus, put an end to most getters/setters for members and instead focus on having objects do something for me and give me the fruits of their labor?

Nice to say but in the real world it often doesn't work that way. DAO objects are a good example. What do you want a DAO to do other than give you access to data so you can apply business rules to it? Or how would you use the Date class without getters and setters?

Originally posted by Thomas Paul:
Nice to say but in the real world it often doesn't work that way. DAO objects are a good example. What do you want a DAO to do other than give you access to data so you can apply business rules to it?

The important word is *most* - it's a heuristic, not a law. A factory object like a DAO certainly needs a kind of get method.

Or how would you use the Date class without getters and setters?

That, of course, depends on what you are using the accessors for.
Again, it wouldn't be reasonable to make a rule like "don't use accessors". It makes *much* sense to me, though, to have a rule "if you feel the need for an accessor, think hard about an alternative design". Your milage may vary...

IP: Instead, he proposes to fully hide the data and expose behaviour instead. And I think he is right in doing so.
I don't disagree with his point that you can have too many accessors. But accessors are NOT inherently non-OO, and they are NOT the same as public fields.
He's hyping his new book by being alarmist, and people who do that in programming should be strung up by their heels.

Here is a typical line of code:
if (someObject.getSomeAttribute() == someValue)
{
doSomething();
}
This is a case where my behavior changes based on the state of a contained object. This is a standard pattern in business, and something I do over and over. In order to move this code into "someObject", it must know about me, and specifically it must know about doSomething(). I prefer my objects to have as little interaction as possible, and wherever possible, knowledge of one object by another to be one-way.
I could almost do this if I had callbacks. That is, if I could pass the address of doSomething() to someObject and let it determine whether or not to call it, but that's difficult. Or I could add a method which returns a boolean for every possible value of an attribute, but obviously that would only work for enumerated values. So, in this case, by far the best concept is to use a getter.
Personally, I think time spent worrying about removing getters is time wasted.
Joe

Originally posted by Joe Pluta:
Here is a typical line of code:
if (someObject.getSomeAttribute() == someValue)
{
doSomething();
}

You can formulate this without using a getter easily:

I could almost do this if I had callbacks. That is, if I could pass the address of doSomething() to someObject and let it determine whether or not to call it, but that's difficult.

Actually, it's not:

Granted, it is a little bit verbose, but only because of the shortcomings of Java. In Smalltalk, it looks much better:

In fact, it's exactly as long as the if statement, but more readable:

Wether that's actually better (or wether a different better solution exists) will of course heavily depend on the actual code. There certainly *are* cases where the accessor is the best you can do. Again, avoiding accessors is a heuristic, not a rule.

Personally, I think time spent worrying about removing getters is time wasted.

I tend to wholeheartedly disagree...

JavaRanch ate my previous post. An unbalanced HTML tag somewhere. And when I hit the back button everything was gone. So, instead I'm just going to summarize:
State checkers like "isLessthan" don't buy you much from a pure OO standpoint. If you have named methods for every possible condition, you actually do provide a little information hiding, but it's impractical.
I would never implement your IBlock approach unless it was absolutely necessary. The overhead of instantiating an object and going through a nested call is far too high a price to pay for "elegant" code.
In short, your concepts are interesting, but I don't see them as providing any benefit to an average program, and I, if anything, am an average programmer.
Joe

Great give and take, gang! I agree that "evil" is a bit alarmist but I like some of the things we all got from this. Adding a getter reveals internal structure which is an opportunity for increased coupling. Every time you add one, stop and ask yourself if you couldn't encapsulate the behavior instead. Some times you can, some times you can't.

Originally posted by Joe Pluta:
JavaRanch ate my previous post. An unbalanced HTML tag somewhere. And when I hit the back button everything was gone.

I hate it when that happens.

State checkers like "isLessthan" don't buy you much from a pure OO standpoint. If you have named methods for every possible condition, you actually do provide a little information hiding, but it's impractical.

Granted, "isLessThan" wouldn't buy you anything. What I would aim for is a less structure revealing, more abstract name. Perhaps something along the lines of "isValidFor". But it's hard to discuss this on such an abstract example - again, a real world example would help.

I would never implement your IBlock approach unless it was absolutely necessary.

I was sceptical at first, too - but I like to experiment.

Now that I am somewhat used to it, I find it to be superior most of the time. YMMV.

The overhead of instantiating an object and going through a nested call is far too high a price to pay for "elegant" code.

With all due respect, that's nonsense. It's typically nothing to worry about.

In short, your concepts are interesting, but I don't see them as providing any benefit to an average program, and I, if anything, am an average programmer.

Well, so we still disagree. That's ok, I suppose.

While I tend to agree with Ilja's point of view, I have to concede that sometimes it's harder to code/think that way. There are many situations where your code is more naturally what I like to call "reactionary" vs. being "delegatory". I can't think of a concrete example but this analogy comes to mind:

It would not be practical to have aCar.steerClearOfYou(), would it? Or perhaps you could honkAt(aCar) and hope that it performs changeDirection() in a timely manner so as to avoid a headOnCollisionException

.
Hey, I think I just convinced myself that there might be other ways after all, if you think hard enough...or maybe that wasn't such a good analogy. Maybe most of us are just not used to thinking that way.
Wait, let me see if I can do another one:

Now that seems like perfectly fine "reactionary" code, doesn't it?
Like Ilja said, it's a heuristic and probably the best we can do is to rely on experience and come up with a good set of guidelines to help us decide when it's appropriate or not to use getters/setters.
My $0.02
---
Edit: That first example really wasn't a good one, was it? How would aCar.isComingAtYou() work? It would probably be more like if (isComingAtYou(aCar)) but then again you would have to have stuff like aCar.getDirection() and aCar.getVelocity()... hmmm

[ September 10, 2003: Message edited by: Junilu Lacar ]

And Ilja, let me say that I don't think your concepts are invalid. In fact, I will add "review for unnecessary getters" to my code review.
At the same time, I disagree with your statement that it's "nonsense" to worry about adding an instantiation to every comparison. By far the biggest knock on Java applications is that they are slow, and adding this amount of overhead to a comparison is one way to continue those complaints.
Just a quick example:

public class Tester
{
    private static final int LOOPCOUNT = 100000000;
    
    class IBlock {
        public void perform() { return; }
    }
    
    class Inside {
        int attr = 0;
        public void setAttr(int newAttr) { attr = newAttr; }
        public int getAttr() { return attr; }
        public void test(IBlock ib) { if (attr == 1) ib.perform(); }    
    }
    public static void main(String[] args)
    {
        new Tester().run();
        System.exit(0);
    }
    
    public void run()
    {
        Inside in = new Inside();
        
        long time1 = System.currentTimeMillis();
        for (int i = 0; LOOPCOUNT > i; i++)
        {
            if (in.getAttr() == 1) doit();
        }
        
        long time2 = System.currentTimeMillis();
        for (int i = 0; LOOPCOUNT > i; i++)
        {
            in.test(
            new IBlock() { public void perform() { doit(); } });
        }
        
        long time3 = System.currentTimeMillis();
        
        System.out.println("Test 1: " + (time2-time1) + ", Test 2: " + (time3-time2));
    }
    
    public void doit() {}
}

Run this and see what kind of results you get. Depending on your environment, you could see as much as a 4000% jump in runtime for the second test. Now, this is because I have to instantiate the object every time. If I pre-create the object, times drop considerably, but that's my point - you have to think ahead before implementing any architectural decision.
There needs to be a common sense guideline, and I think that's in each developer's mind. IMO, you should always keep performance somewhere in your thoughts.
And no matter what, after reading your comments here, I will never look at getters the same way. Despite my original comment, from now on I will always check and see whether a getter is necessary. My definition of "necessary" may be a little different from yours, but that's okay - that's why programming is done by people, and not machines!
Thanks very much for opening my eyes a little - and now I have to ice down my head

.
Joe

Originally posted by Joe Pluta:
Run this and see what kind of results you get. Depending on your environment, you could see as much as a 4000% jump in runtime for the second test.

I got roughly 3000%.
But you also need to take into account how much time a typical business application spends in this type of code. By just adding a "in.setAttr(i % 2);" inside the loops and "Math.pow(1.0, 3.3);" into doit(), the difference already dropped to 200%.

If I pre-create the object, times drop considerably, but that's my point - you have to think ahead before implementing any architectural decision.

I think you just proved that you *don't* need to *act* ahead, but can still optimize when it proves to be a problem.

There needs to be a common sense guideline, and I think that's in each developer's mind. IMO, you should always keep performance somewhere in your thoughts.

I agree - that's always a good idea. But I would always act on design considerations before performance considerations. That's because it's maintenance of the code which is the big cost factor. And actually well designed code is way easier to optimize later!

And no matter what, after reading your comments here, I will never look at getters the same way. Despite my original comment, from now on I will always check and see whether a getter is necessary. My definition of "necessary" may be a little different from yours, but that's okay - that's why programming is done by people, and not machines!

Originally posted by Junilu Lacar:
It would not be practical to have aCar.steerClearOfYou(), would it? Or perhaps you could honkAt(aCar) and hope that it performs changeDirection() in a timely manner so as to avoid a headOnCollisionException .

Or you could simply implement the CarObstacle interface and register yourself at the car...

if (trafficLight.isGreen()) { go(); } 
Now that seems like perfectly fine "reactionary" code, doesn't it?

Wouldn't it be much more convenient if the trafficLight simply noticed you when it got green (Observer pattern)?

Originally posted by Ilja Preuss:
I think you just proved that you *don't* need to *act* ahead, but can still optimize when it proves to be a problem.

HORRORS!
I hope I didn't do that! This is a trivial case; if you coded it wrong from the start, then used that as a template for a lot of classes, and only THEN found out it was a problem, then you'd have to go back and fix it everywhere! It's like anhy other design consideratin.
My point is that performance is not "in addition to" or "instead of" other design considerations - it needs to be part of the design! Performance has to be AT LEAST as important an up-front design consideration as "elegance". I may be an old fogie, but from my experience, no coding technique is inherently any better than any other (all can be implemented well, or poorly), and design philosophies come and go. But performance and end users expectation are a constant: it's ALWAYS too slow

.
Code bloat is a major factor in the slow uptake of Java, especially on the desktop, and to relegate performance to an afterthought is a sure way to promulgate bloatware.
Joe

Originally posted by Ilja Preuss:
By just adding a "in.setAttr(i % 2);" inside the loops and "Math.pow(1.0, 3.3);" into doit(), the difference already dropped to 200%.

I love it. I can't help myself: "200% here, 200% there, and pretty soon you're talking REAL performance problems"

.
Anyway, you're an extreme programmer, Ilja, and so you and I are going to be way on opposite sides of the fence here. You think performance can be modified into a system, and I think you have to design your system from the beginning with performance in mind.
I can only go by my own experience, but that's one thing us old fogies have: lots of experience

. That doesn't mean we can't learn a few new tricks, though, so please keep those comments coming!
Joe

Originally posted by Joe Pluta:
I hope I didn't do that!

I still think you did!

This is a trivial case; if you coded it wrong from the start, then used that as a template for a lot of classes, and only THEN found out it was a problem, then you'd have to go back and fix it everywhere!

If you really used it as a template - like in the Template Method pattern or Template For My Code Generator (you get the idea) - no, you'd still had to fix it only at one place.
If instead you "template" as in Rape And Paste Programming - well, that would be HORRORS to me!

It's like anhy other design consideratin.

Excactly - hold the code flexible by removing duplication and encapsulating every important decision in exactly one place.
That is where I would agree that you *should* *think* about performance considerations - just don't optimize for performance at those places, but for flexibility/optimizability.

My point is that performance is not "in addition to" or "instead of" other design considerations - it needs to be part of the design! Performance has to be AT LEAST as important an up-front design consideration as "elegance".

Well, I'd rather measure the bottlenecks instead of guessing about them - especially with modern processors and Hot Spot engines. And until I measured them I'd rather have the best design I can think of, so that performance is easy to measure and optimize. Your mileage may vary.

But performance and end users expectation are a constant: it's ALWAYS too slow .

So I need the most effective technique for performance optimization, correct?
I wonder which one that might be:
- Splattering performance optimization over the whole code from the beginning based on heuristics, thereby slowing down development because of compromising design, or
- Concentrating on design and only using the end of the project (where we do have time saved from not optimizing prematurely) searching the most important bottlenecks (which with some probability wouldn't have been handled by the heuristics) and getting a vast improvement by only touching a small amount of code.
Of course there is actually a balance between the two extremes, but I think the latter is the much more important strategy.

Code bloat is a major factor in the slow uptake of Java

Is it? Where do you get this from?
BTW, I currently have to analyze a performance problem in a software product we just got a maintenance contract for. I will post here about my findings of the cause, if you are interested.

Originally posted by Joe Pluta:
I can't help myself: "200% here, 200% there, and pretty soon you're talking REAL performance problems" .

That's not my experience. I also think the benchmark still isn't very representative for real world applications.

Anyway, you're an extreme programmer, Ilja, and so you and I are going to be way on opposite sides of the fence here. You think performance can be modified into a system, and I think you have to design your system from the beginning with performance in mind.

I don't think this has anything to do with XP. Tony Hoare said "Premature optimization is the root of all evil", and Donald Knuth restated it. Googling for "rules of optimization" will also give you many sites stating something going in the same direction, and many aren't connected to XP at all (many even pre-XP).

I can only go by my own experience, but that's one thing us old fogies have: lots of experience . That doesn't mean we can't learn a few new tricks, though, so please keep those comments coming!

Ah, it's not so easy to stop me!

Originally posted by Ilja Preuss:
I don't think this has anything to do with XP. Tony Hoare said "Premature optimization is the root of all evil", and Donald Knuth restated it. Googling for "rules of optimization" will also give you many sites stating something going in the same direction, and many aren't connected to XP at all (many even pre-XP).

Forgive me if, as someone who has designed and developed successful commercial products for over 20 years, I don't make my decision based entirely on what Profs. Hoare and Knuth say

. I have learned over the years that academic programming and commercial programming typically have very different goals. I've found that someone who has never had to worry about the pressures of delivering working code to end users sometimes has a somewhat unrealistic view. Often the design outweighs the deliverables, and that leads to programmers who, while brilliant, fail miserably in the field.
At the same time, chronic workaday guys like me need to learn as much as we can from academia, so that we can apply as many of those wonderful nuggets of knowledge as is economically feasible.
Remember, programming is not about some Platonic ideal. In the end, it's about making the end user happy. If you can do it with BASIC and a teletype, then you've succeeded. And since the end user is always concerned about performance, to simply poo-poo a 200% performance degradation that is immediately obvious to a common sense review of the design is fiscally irresponsible. So I guess my point is that, when you're designing your system you should always be thinking "will this design choice really cause major performance issues?" because when it come down to it there are no perfect designs, except for the ones that make the end user perfectly happy.
And so Q.E.D, there are no perfect designs

.
The single most important rule of programming, and indeed most endeavors: "The perfect is the enemy of the good."
Joe

The only absolute is that there are no absolutes.
Getters and setters are tools. They are not philosophical necessities, nor demon spawn. You use them when they make sense, not because some book told you that you should. If you aspire to become journeyman or master software developer, then you need to understand the principles behind the practices.
There is a difference between an object and a data structure. Objects are abstractions of behavior, that imply the existance of some data, but don't expose it directly. Data structures are physical representations of data, and imply no behavior at all. Putting getters and setters in either is a failure of some kind.
Getters and setters in a data structure are useless. In a data structure the fields should be public and access should be direct. In an object, getters and setters expose the implied data and compromise the behavioral abstraction.
Thus, you should always feel uneasy when using getters and setters. This doesn't mean that you won't use them. You will. Software is not about philosophical truth, it's about creating business value for our clients and managers. There will be times when you will put getters and setters into an object -- and this will be an admission that something is philosophically imperfect. There will also be times when you put getters and setters into a data structure, and again, this will be an admission that something is philosophically imperfect. And that's OK. We aren't after philosophical perfection. We are after high quality, clean code, and business value.

Originally posted by Robert Martin:
Getters and setters in a data structure are useless. In a data structure the fields should be public and access should be direct.

Nothing is absolute. Public fields are nice in a perfect world, but in complex, multi-person development efforts, I have found it helpful to hide all details, including those of data structures. This prevents inadvertant modifications, and gives us auditability. This is crucial when something breaks. This also allows us to put in whatever data integrity we want when a field is set.
It's a programming, not philosophy, and what's right is what delivers good code.
Joe

Originally posted by Joe Pluta:

Nothing is absolute.
Joe

This is more often true with choices we make than with consequences of a given action because what we want changes, but the results of a particular action with particular variables does not.

Originally posted by Joe Pluta:
Forgive me if, as someone who has designed and developed successful commercial products for over 20 years, I don't make my decision based entirely on what Profs. Hoare and Knuth say .

Nothing to forgive - I would be alarmed if you would! Just notice that it's not an XP-thingy.

Remember, programming is not about some Platonic ideal. In the end, it's about making the end user happy.

Or rather those who are paying for the software. Agreed.

And since the end user is always concerned about performance, to simply poo-poo a 200% performance degradation that is immediately obvious to a common sense review of the design is fiscally irresponsible. So I guess my point is that, when you're designing your system you should always be thinking "will this design choice really cause major performance issues?" because when it come down to it there are no perfect designs, except for the ones that make the end user perfectly happy.

But what the most customers are often even more concerned about are costs. So I think it's reasonable to only sacrifice time for those optimizations the customer explicitly cares about, and otherwise optimize my design for development speed.

Originally posted by Ilja Preuss:
But what the most customers are often even more concerned about are costs. So I think it's reasonable to only sacrifice time for those optimizations the customer explicitly cares about, and otherwise optimize my design for development speed.

You've got a point, Ilja. I'm going to take that into consideration during my next round of code reviews.
So let's apply it to the current situation. What do you think makes the IBlock approach superior to the getter as far as development speed? Why do you think it's faster to add another class as well as a testing method to the object, as opposed to a simple test to a retrieved attribute in the caller?
Also, just a quick question - when the callback method is invoked from the called object, are the current instance variables of the calling object still visible? Common sense says yes, but I could see where it could be tricky for the compiler.
Joe

Originally posted by Joe Pluta:
You've got a point, Ilja. I'm going to take that into consideration during my next round of code reviews.

So let's apply it to the current situation. What do you think makes the IBlock approach superior to the getter as far as development speed? Why do you think it's faster to add another class as well as a testing method to the object, as opposed to a simple test to a retrieved attribute in the caller?

Well, I have to admit that I am still experimenting with this approach.
My current feeling is that at least in some instances, the block approach provides better means of abstraction and decoupling. After getting used to it, it's even sometimes more readable to me.
There are probably cases where the more direct approach is better. In the case of our rather abstract examples, I really couldn't say that one would be better than the other...

Also, just a quick question - when the callback method is invoked from the called object, are the current instance variables of the calling object still visible? Common sense says yes, but I could see where it could be tricky for the compiler.

The compiler basically creates a toplevel class with a reference to the outer object, so yes, the instance variables are still accessible. (For otherwise non-accessible fields, special accessor methods are created.) You can also access final local variables (the inner object just gets copies of them at creation time).