imho, it's not bad design. though it doesn't follow convention. what if, in the you need to provide pre/post processing in the getter and/or setter.
–
xandercodedAug 8 '12 at 0:11

Why would you have a setter if you say they will never change?
–
axlAug 8 '12 at 0:14

1

However you do it, be consistent. As far as using OO design as your gauge - POJOs alone aren't exactly the paragon of OO.
–
Matt BallAug 8 '12 at 0:15

Premature optimization is the root of all evil. -- Dr. Donald Knuth. Although Dr. Knuth was referring to performance optimizations, the same can be said of coding practices that add clutter without adding value.
–
Peter GluckAug 8 '12 at 0:23

@PeterGluck I read that as: a) limit scope until you need to increase it - b) don't put setters, until you need them - c) make fields final, until you need to change them?
–
assyliasAug 8 '12 at 0:26

8 Answers
8

Unfortunately, good (or bad) design is 100% dependent on how it is going to be used.

As a rule of thumb, it is a good idea to keep member variables private, just so that the Object model is in control of their access. This implies the first approach is better.

BUT

if the values never change, what's the point? Why bother writing setters if they will never be used?

So, which one is better? As I mentioned above, that depends on what you are doing this for. If it's for an assignment for class, I would go with the first one. Your teacher will like that more, as it is more "textbook".

So, if this is a personal project or for work where you can take advantage of future releases, I would go with a cross between the two:

This approach is safe, because the member fields are private, and there isn't the "code smell" of unused methods. This is also fairly extensible, as you can quite easily add the setters if your requirements ever change and you need to modify the fields.

Getter and Setter make API more stable. For instance, consider a field public in a class which is accessed by other classes. Now later on, you want to add any extra logic while getting and setting the variable. This will impact the existing client that uses the API. So any changes to this public field will require change to each class that refers it. On the contrary, with accessor methods, one can easily add some logic like cache some data, lazily initialize it later. Moreover, one can fire a property changed event if the new value is different from the previous value. All this will be seamless to the class that gets value using accessor method.

ThePragmaticProgrammer recommends always using getters and setters too, because it is the more general interface. Check this article http://c2.com/cgi/wiki?AccessorsAreEvil it's very nice and gives deep understanding on why we should use them.

In general, just make every (instance) field private (exception for Data Transfer Objects). If you have setters, then you've probably abandoned all hope for encapsulation. Getters are fine for immutable value objects. ("get" prefix is a convention (mostly) for Java, but it's largely noise, particularly in code that favours immutable value objects and tell-don't-ask interfaces. If you remove the "get" the client code only has the extra "()" as verbosity over direct field access.)

There's some good answers out there, and I agree with making fields private and adding appropriate constructors/getters/setters to limit mutability where needed.

I will point to another drawback of using field based assignment. Any proxy based AOP will have trouble adding pointcuts to field assignments, so it would make it difficult to do logging (and other operations) via proxy based AOP constructs, as they are limited to method interception. You'd be forced most likely into actually doing weaving, which becomes more complicated. From Spring's own documentation:

Spring AOP currently supports only method execution join points
(advising the execution of methods on Spring beans). Field
interception is not implemented, although support for field
interception could be added without breaking the core Spring AOP APIs.
If you need to advise field access and update join points, consider a
language such as AspectJ.