views:

2744

answers:

19

I am sick and tired of getting yelled at when I make all my variables in a Java class public, for the sake of not needing to write five times more code for getters and setters.

Do others share this problem?

+1  A: 

Take it in a constructive manner, the person yelling may have been burned badly by it before, or having a bad day, and rarely, may have bone to pick with you. Learn from it, try not to repeat it, and move on. We all have only one life.

Merry Christmas!

omermuhammed
+94  A: 

No, I'd be one of the ones doing the yelling.

Encapsulation is important.

Admittedly C# 3.0 makes it easier than it is in Java, but even so I always do it the right way rather than taking shortcuts for the sake of short-term productivity at the cost of long term maintainability.

See also "private vs public members in practice",

Jon Skeet
Do you emphatically apply this philosophy to ALL objects, no matter the form or function? What about DTOs, for example?
Chris Noe
Occasionally I'll have a non-private variable within a private nested class, as then at least the visibility is still restricted to the same outer class. Other than that, I can't remember writing a non-private variable and being happy about it.
Jon Skeet
I used to (before automatic properties) sometimes make fields public for things like classes used in tests and inner classes, but I don't see a need for it anymore.
tvanfosson
But if he uses getters and setters for every private variable, encapsulation is gone down the drain anyway.
Nemanja Trifunovic
Encapsulation is definitely not the same thing as getter/setter methods.
alex strange
@Nemanja: Excellent (and oft-ignored) point. @alex: doesn't appear to stop anyone from insisting on the latter when they really want the former...
Shog9
@Nemanja: It is most definitely not gone down the drain, because you can change the underlying implementation without the code that uses the getter or setter ever having to know, or care about it. If you make a field public and access it directly, you can never change it's type or anything else
Sandman
@Alex: It's not the same thing, but separating implementation from API is a starting point at least.
Jon Skeet
@Jon: you're just creating an API based on the implementation. Sure, you can go back later and change the implementation... but only if you can still simulate the old one via the API. It's only a starting point if you're then diligent about factoring usage patterns back into class methods.
Shog9
@Shog9: Yes, there's obviously more to encapsulation than *just* turning fields into properties. I'd say avoiding public fields is a necessary but not sufficient condition for encapsulation.
Jon Skeet
LOL @ "I'd be one of the ones doing the yelling."
Quibblesome
If this guy has so many fields that writing getters is problematic, then it's safe to assume he's violating encapsulation anyway.
Outlaw Programmer
@OP, You've spent more time asking this question than you ever would have writing a getter/setter.
Joe Philllips
+9  A: 
m3rLinEz
In 6.5 is better, we have more options =D
José Leal
+42  A: 

There are really two issues here: The first is a team issue – why do you persist in not following team coding conventions even after having to go back and change your code more than once?

In a team environment, its pretty rare to be able to code just anyway you want to. There will always be things that the team does on way, that you would like to do differently.

The second is technical – there are many reasons to prefer properties instead of "naked" public variables. This is especially true for classes that provide functional boundaries between components, or logical sub-systems. Secondly, you mentioned "all my variables". This is almost always the wrong thing to do.

My recommendation is that you stick with team practices and accepted best practices. Look at it from your manager's and your team members perspective – they may write a question like this: "What do you do with a team member who insists on always making all their Java class variables public? He always gets yelled at for this in code reviews but does it anyway!"

Foredecker
The "what do you do with a team member..." question has been asked and answered multiple times. It usually boils down to firing them.
Chris Lively
Chris - that is so the wrong answer. Firing someone is a last resort - necessary in some instances, but a last resort. This should never be the default answer or what things "boil down to". With all due respect - that's cop out.
Foredecker
+1  A: 

Look into using code templates to automate such simple tasks.

Todd Smith
+8  A: 
dulaneyb
Making too many getters and setters is also not necessarily a good solution, because some state information should stay private.
Uri
True but I believe pretty much all of these code generators let you pick and choose which fields you want to generate code for.
Outlaw Programmer
+16  A: 

To be as fair as possible, there is a school of thought that "pure data" DTOs which are nothing more than carriers of data may be reasonably expresses with public final data. For example (pardon the wheel-reinvention for sake of discussion):

public class Coordinate2D {
    public final double x;
    public final double y;
    public Coordinate2D(double x, double y) {
        this.x = x;
        this.y = y;
    }
}

instead of a corresponding definition using private fields and get... methods.

However, even if one accepts that argument, there are still a number of drawbacks. (I promise not to yell! ;-)

  1. This only applies to DTOs, which should at most exist at the "edge" where an OO system must talk to non-OO artifacts (e.g. database, message exchange with a legacy system, etc.) and shouldn't be normal practice within the OO codebase. Properly designed object should expose behavior (perform services for other objects), but not their internal state, and certainly not their representation of their internal state. OO is about optimizing for flexibility, not optimizing for speed at the cost of all else.

  2. Allowing fields to be non-final (including the use of set... methods) can cause all manner of problems WRT concurrency and/or consistency constraints. As an example of the latter point, think of an Address containing a city name, state, and postal code (called ZIP Code (tm) in the US, for the USPS Zone Improvement Plan). If Address allows mutation of city name, then the consistency relationship between city and postal code may be broken, unless the postal code is also updated (or at least verified).

  3. Even in the case of DTOs, a change of representation can cause problems if knowledge of the representation is allowed to leak. Back to our coordinate example above, a suitably-designed coordinate class could use polar representation internally, while continuing to support clients that only knew about getX() and getY().

  4. Allowing this exception for DTOs means that other parts of the system have to be written with the knowledge of which classes define DTOs and which do not. Inconsistencies such as that tend to cause long-term problems with ongoing maintenance (what if a boundary moves?) and require the developer to keep One More Exception in mind throughout the entire design/development/review/maintenance lifecycle.

  5. Some types are inherently more difficult to make immutable. A field of type char[] can be final, preventing the entire array from being replace, but that still allows the content of the array to be modified. In the cases where immutability is important, a get... method can return a copy of an array, can wrap a collection in an unmodifiable equivalent, etc. to preserve the safety and consistency of the private data.

So, on balance, although I tried hard to make a case to myself for immutable public fields, I ended up concluding that it was more trouble than it was worth for me.

joel.neely
+6  A: 

Another way to look at this issue is from a theoretic standpoint:

Imagine all the code in your project as part of one large API. Imagine that every developer needs to understand as much of the API as possible. Every time you declare anything public, or when you create getters and setters for internal state that isn't really needed, you add more to the API, making it less likely that someone would understand all of it.

Of course, the real problem is that public things add the potential for more constraints. Somebody may just use your field, which constrains whether you can change it later, etc.

Also, my own research suggested that the longer the list of services, the more likely people are to not notice important services or to create redundancies. By offering useless things to your clients, you hide the useful stuff. Can't see the forest for all the trees.

Uri
+6  A: 

Getters and setters are evil.

People can talk about encapsulation all day, but in Java they are only really needed if:

  • The getter and setter are not "standard" or have a fair chance of someday not being so. i.e.: the setter does not exist, they do not assign/return a field directly, etc. Notes:
    • Funnily enough, non-standard setters and getters seem like a source of obfuscation
    • If they are never going to be non-standard, you just have written more code for no benefit
    • "Fair chance" is subjective. I would adjust this depending on the "public-ness" of the object in question. If this is a class that will be published as an API for lots of code to use, you should probably lean more towards using getters and setters. If this is an internal class, you should not.
    • Getters that return copies of mutable objects with no setter are nice
  • You are declaring/implementing an interface
  • You are interacting with something that needs JavaBean properties (i.e. you cannot read a public field in a JSP, you need a getter)

Other ideas:

  • You should follow existing code conventions in your project or change them formally, you should not ignore them
  • Almost no "best-practice" is always the best practice.
alex
Just to be sure, you mean they are evil, but just a little less evil than public member variables, right? The evil part is exposing your implementation, not writing extra code!
Bill K
That's the interpretation I had when I upvoted this answer.
David Thornley
They are evil because people automatically use them without thought. Getters and setters have their use, but so do fields.Writing extra code is not evil (my IDE does it for me), but having extra code is (more maintenance overhead).
alex
+3  A: 

Why are you in such a hurry? How much time will you save doing this? 5 minutes? Would you want your car mechanic to save 5 minutes on a job or would you want him to spend the extra 5 minutes making sure those bolts are really tightened properly!?

Java is a verbose language and doesn't have properties in the way that (say) c# or Delphi does.

I suspect that the people doing the shouting have been burned before and are trying to avoid problems down the road. Ask them why they want it done this way and listen. They may be right, they may not be right but at least hear them out.

The key to getting along with your team is to learn their idioms and follow them (where they make sense). You should also try and follow the language idioms as well, after all... you don't want to be known in 2-3 years as the guy that did things his own way!

FWIW I use Eclipse to generate getters and constructors. I always generate toString, equals, hashcode and compareto methods as a matter of course. These things help with debugging, collections usage and (most importantly) concurrency. It takes just a few minutes to do this.

After all, a final class that is immutable is (generally) threadsafe.

Fortyrunner
+1  A: 

There are 2 reasons to have getters and setters:

  1. The spec changes to make the attribute either read or write only
  2. The implementation changes. Example:

    class Person {
        private string name;
        public string GetName() {
            return name;
        }
        public string SetName(string value) {
            name = value;
        }
    }
    

    is changed to:

    class Person {
        private string firstName;
        private string lastName;
        public string GetName() {
            return firstName + " " + lastName;
        }
        public string SetName(string value) {
            string[] temp = value.split();
            firstName = temp[0];
            lastName = temp[1];
        }
    }
    

The example is not a breaking change, having to get all clients to change to the getters and setter would be.

Although the first reason I gave would break the API, it is still less severe than changing from variable access to getters and setters, only clients that set the attribute are affected.

Martin Hornagold
I would say that in some ways this change is MORE severe. If you change the API, all the clients won't compile. If you keep the same signatures but change the contract, things will break silently. I see what you're saying but this looks like a bad example.
Outlaw Programmer
How is the contract changed?Both versions get and set a name.How that name is stored in memory is irellavant to the user.
Martin Hornagold
+4  A: 

In the first place, learn the coding standards where you are and follow them. If you can't follow that simple principal, you're not fit to work in any decent shop. Willful and/or repeated defiance should be considered grounds for firing for cause.

In the second place, getters and setters are better than raw access. They can provide a more stable interface, and provide hooks if you ever need them. I remember one very important change I was able to make because all changes to a variable went through a setter, so I could provide a bit of additional functionality there. To some extent, they allow you to change the implementation without changing the interface.

In the second place, why is it necessary to make every single variable independently changeable? A class should be more than a random collection of data. It should represent something, and its member variables should be consistent. You can't enforce any sort of consistency if all the member variables can be arbitrarily changed. You should define how the class is to be used, and provide member functions to do all of that. This allows you, or somebody down the line, to change the implementation as desired (maybe it's interfacing with a different external component, or it needs to be more efficient, or it just needs to work differently), without having to track down and rewrite everything that uses the class.

If you think it's reasonable to expose all class variables to getters and setters, you need a lot more mentoring in OO programming and design. I'd consider you useless in OO programming right now. (Remember that we all start out useless. The best we can hope for is the ability to learn.)

So, if you worked here, I'd yell at you for your disregard of coding standards. Once that was settled, I'd work with you to help you understand what you're doing, until I was comfortable letting you touch production code without close supervision.

David Thornley
+2  A: 

This is a minor view, but I think getters and setters are a waste of code space most of the time as they are trivial and the problems they are designed to solve can be easily solved with refactoring most of the time. This is rather a minimalist view to coding is really about coding style.

Given most people prefer to see getters and setters and I suggest using getters/setters for interfaces and public classes. i.e. anything you expect other people to use. Also they are a good idea if your class has non-trivial logic. I'll admit this is not a rule I always follow myself, but probably should.

However if you have a simple data/value struct-like class, then I find getters and setters a mindless addition. I know they are easy to generate, so is a lot of pointless code.

BTW: Have a look at ByteBuffer for a different style of getters and setters I quite like, but again most people dislike this approach too.

Peter Lawrey
A: 

If you have JavaBeans then getters and setters are standard. However if you have POJOs getters and setters should only appear based on an actual requirement. http://en.wikipedia.org/wiki/POJO

Peter Lawrey
+1  A: 

Encapsulation is important. You need to write the getters and setters.

However, your frustration with writing getters and setters over and over again is justified.

MJD argues in his article Design Patterns of 1972, that patterns are often evidence of weakness in the language. The design patterns of 1972 are called 'functions' or 'objects' today and are features of the language. In other words, if you find yourself writing code over and over again, whether it be singletons or getters and setters, it may be a problem your language of choice has failed to solve.

Some people are solving this problem with their editor, but I feel that object systems like Moose, perl6, and apparently C# have solved this problem more completely so that you never have to write a getter or setter unless its nontrivial.

Eric Johnson
C# only requires non-trivial getters and setters."Perfection is reached, not when there is no longer anything toadd, but when there is no longer anything to take away." -- Antoine de Saint-Exupery
Peter Lawrey
+1  A: 

What sort of place do you work at? I for one would never tolerate anyone yelling at me for the code I write. that's just ridiculous and unhelpful and isn't the right way to get people to change their habits and learn from their mistakes...

Scott Evernden
Okay, do you demand the right to do stupid things repeatedly without consequences? You screw up the code base once, I'll try to work with you so you can see why what you did was wrong. Three or four times and at best I'll be patiently explaining why I completely agree with the yellers.
David Thornley
A: 

I partially agree with you. In many applications the variables need to be public to make sense. Like in a 3D vector class I would get sick and tired of writing vector.getX()/vector.setX() instead of just vector.x. But then again, follow the conventions of your company. If they say you should use private variables, use private variables. I don't see the reason for using only public variables either. If having the variables public isn't needed to keep everyone sane, don't make them public.

martiert
In most applications, most variables should be private, normally without explicit getters and setters (there may be lots of getters, but they will not exist explicitly to allow access to class variables - Balance() in a bank account is to show balance, not the value of m_balance.
David Thornley
+3  A: 

The real issue is, why do you need access to the member variables at all? I realize some amount of information transfer is necessary, but if you make a habit of making setters and getters for all your member variables, you're totally missing the point.

In OO design, you are supposed to ask an object to do some work for you, you aren't supposed to ask it for it's information and do work on that information.

Getters and setters are bad for exactly that reason, not that you have to type more, but that you are exposing your information and therefore tempting another class to operate on the information instead of asking the owner of the information to do the operation for you.

This doesn't apply to "information" beans which I don't believe in anyway--if you don't have any code to go with your information, it should be in a data structure (Hash), not an object with nothing but getters and setters.

Just a quick example. If you had a member age in person, setAge(int years) and int getAge(), might seem obvious methods to define. Another class might if(person.getAge() < 21) sysout("No drinking for you!");

This has problems. First of all, the drinking age is now spread throughout your code--everywhere someone makes this test. It would make more sense to have if(person.isOfAge()), now the age is in one place. Better yet, this is easily factored into if(person.isOfAge(state)). Now Person can ask state if a person of a given age is legal. No setters or getters at all.

Now, you decide later to re-implement it as holding a birthdate instead of an age. What sense does setAge(int) make??? It can't even be implemented.

Bill K
A: 

I'm even surprised at that question.

You don't just make field to properties because these are rules. There are reasons.

  1. Property behavior can change over time. Instead of straightforward setting/getting it may come to some logic inside. If you used properties then the rest of dependent code will stay unaffected by changes.

  2. By not making fields public you protect yourself against yourself. What if you were sleepy and tired and by mistake let some outside code directly assign a class member variable to a value which should never be allowed? If it were a property it could either give you a warning at execution or maybe even at compile-time if it was a one way property (I expect that is possible in java like it is in C# - property X { get; private set; }).

User