views:

582

answers:

14

I have a question about Java style. I've been programming Java for years, but primarily for my own purposes, where I didn't have to worry much about style, but I've just not got a job where I have to use it professionally. I'm asking because I'm about to have people really go over my code for the first time and I want to look like I know what I'm doing. Heh.

I'm developing a library that other people will make use of at my work. The way that other code will use my library is essentially to instantiate the main class and maybe call a method or two in that. They won't have to make use of any of my data structures, or any of the classes I use in the background to get things done. I will probably be the primary person who maintains this library, but other people are going to probably look at the code every once in a while.

So when I wrote this library, I just used the default no modifier access level for most of my fields, and even went so far as to have other classes occasionally read and possibly write from/to those fields directly. Since this is within my package this seemed like an OK way to do things, given that those fields aren't going to be visible from outside of the package, and it seemed to be unnecessary to make things private and provide getters and setters. No one but me is going to be writing code inside my package, this is closed source, etc.

My question is: is this going to look like bad style to other Java programmers? Should I provide getters and setters even when I know exactly what will be getting and setting my fields and I'm not worried about someone else writing something that will break my code?

+1  A: 

Since you're the only one writing code in your closed-source package/library, I don't think you should worry too much about style - just do what works best for you.

However, for me, I try to avoid directly accessing fields because it can make the code more difficult to maintain and read - even if I'm the sole maintainer.

David Underhill
+2  A: 

Most Java developers will prefer to see getters and setters.

No one may be developing code in your package, but others are consuming it. By exposing an explicitly public interface, you can guarantee that external consumers use your interface as you expect.

If you expose a class' internal implementation publicly:

  • It isn't possible to prevent consumers from using the class inappropriately
  • There is lost control over entry/exit points; any public field may be mutated at any time
  • Increase coupling between the internal implementation and the external consumers

Maintaining getters and setters may take a little more time, but offers a lot more safety plus:

  • You can refactor your code any time, as drastically as you want, so long as you don't break your public API (getters, setters, and public methods)
  • Unit testing well-encapsulated classes is easier - you test the public interface and that's it (just your inputs/outputs to the "black box")
  • Inheritance, composition, and interface designs are all going to make more sense and be easier to design with decoupled classes
  • Decide you need to add some validation to a mutator before it's set? One good place is within a setter.

It's up to you to decide if the benefits are worth the added time.

Abboq
+4  A: 

I would adhere to a common style (and in this case provide setters/getters). Why ?

  1. it's good practise for when you work with other people or provide libraries for 3rd parties
  2. a lot of Java frameworks assume getter/setter conventions and are tooled to look for these/expose them/interrogate them. If you don't do this, then your Java objects are closed off from these frameworks and libraries.
  3. if you use setters/getters, you can easily refactor what's behind them. Just using the fields directly limits your ability to do this.

It's really tempting to adopt a 'just for me' approach, but a lot of conventions are there since stuff leverages off them, and/or are good practise for a reason. I would try and follow these as much as possible.

Brian Agnew
As for 2, it's also part of the JavaBean specification.
R. Bemrose
+7  A: 

Even within your closed-source package, encapsulation is a good idea.

Imagine that a bunch of classes within your package are accessing a particular property, and you realize that you need to, say, cache that property, or log all access to it, or switch from an actual stored value to a value you generate on-the-fly. You'd have to change a lot of classes that really shouldn't have to change. You're exposing the internal workings of a class to other classes that shouldn't need to know about those inner workings.

JacobM
Or, you could use AOP for all that cases.
Ondra Žižka
Also, on a mental front, it's easier to grasp a private variable at a glance. You instantly know it's limitations, there is no way this is going to change unless something in THIS FILE changes it. Giving up that ability to save typing a few characters seems criminal--I wouldn't feel comfortable working with someone who did that on purpose--I would have to assume he wasn't thinking of the next programmer AT ALL when he was programming.
Bill K
+1  A: 

Style is a matter of convention. There is no right answer as long as it is consistent.

I'm not a fan of camel, but in the Java world, camelStyle rules supreme and all member variables should be private.

getData();
setData(int i);

Follow the Official Java code convention by Sun (cough Oracle) and you should be fine.

http://java.sun.com/docs/codeconv/

Yada
+1 for link to code convention, I've been needing something like that
jsn
A: 

Even though it's painful, coding up properties with getters and setters is a big win if you're ever going to use your objects in a context like JSP (the Expression Language in particular), OGNL, or another template language. If your objects follow the good old Bean conventions, then a whole lot of things will "just work" later on.

Pointy
+1  A: 

I would be very loath to go into a code review with anything but private fields, with the possible exception of a protected field for the benefit of a subclass. It won't make you look good.

Sure, I think from the vantage point of a Java expert, you can justify the deviation from style, but since this is your first professional job using Java, you aren't really in that position.

So to answer your question directly: "Is this going to look like bad style?" Yes, it will.

Was your decision reasonable? Only if you are really confident that this code won't go anywhere. In a typical shop, there may be chances to reuse code, factor things out into utility classes, etc. This code won't be a candidate without significant changes. Some of those changes can be automated with IDEs, and are basically low risk, but if your library is at the point where it is stable, tested and used in production, encapsulating that later will be regarded as a bigger risk than was needed.

Yishai
Thanks, the direct answer is what I was looking for. There are a lot of really good reasons to use getters and setters listed in other answers. I had my own reasons for breaking encapsulation and wanted to know if this was going to be something other Java programmers are really going to care about.
jsn
A: 

I find getters and setters are better way to program and its not about only a matter of coding convention. No one knows the future, so we can write a simple string phonenumber today but tomorrow we might have to put "-" between the area code and the number, in that case, if we have a getPhonenumber() method defined, we can do such beautifications very easily. So I would imagine, we always should follow this style of coding for better extensibility.

Shamik
+3  A: 

I don't think a good language should have ANY level of access except private--I'm not sure I see the benefit.

On the other hand, also be careful about getters and setters at all--they have a lot of pitfalls:

  • They tend to encourage bad OO design (You generally want to ask your object to do something for you, not act on it's attributes)
  • This bad OO design causes code related to your object to be spread around different objects and often leads to duplication.
  • setters make your object mutable (something that is always nice to avoid if you can)
  • setters and getters expose your internal structures (if you have a getter for an int, it's difficult to later change that to a double--you have to touch every place it was accessed and make sure it can handle a double without overflowing/causing an error, if you had just asked your object to manipulate the value in the first place, the only changes would be internal to your object.
Bill K
+1  A: 

To be brief, you said "I'm asking because I'm about to have people really go over my code for the first time and I want to look like I know what I'm doing". So, change your code, because it does make it look like you do not know what you are doing.

The fact that you have raised it shows that you are aware that it will probably look bad (this is a good thing), and it does. As has been mentioned, you are breaking fundamentals of OO design for expediency. This simply results in fragile, and typically unmaintainable code.

Robin
A: 

Breaking encapsulation is a bad idea. All fields should be private. Otherwise the class can not itself ensure that its own invariants are kept, because some other class may accidentally modify the fields in a wrong way.

Having getters and setters for all fields is also a bad idea. A field with getter and setter is almost the same as a public field - it exposes the implementation details of the class and increases coupling. Code using those getters and setters easily violates OO principles and the code becomes procedural.

The code should be written so that it follows Tell Don't Ask. You can practice it for example by doing an Object Calisthenics exercise.

Esko Luontola
A: 

Sometimes I use public final properties w/o get/setter for short-living objects which just carry some data (and will never do anything else by design).

Once on that, I'd really love if Java had implied getters and setters created using a property keyword...

Ondra Žižka
+2  A: 

I wouldn't care much about the style per se (or any kind of dogma for that matter), but rather the convenience in maintainability that comes with a set of getter/setter methods. If you (or someone else) later needed to change the behavior associated with a change of one of those attributes (log the changes, make it thread-safe, sanitize input, etc.), but have already directly modified them in lots of other places in your code, you will have wished you used getter and setter methods instead.

Kenji Kina
+1 for the thread safety/concurrency aspect
Harold L
A: 

Using encapsulation is a good idea even for closed source as JacobM already pointed out. But if your code is acting as library for other application, you can not stop the other application from accessing the classes that are defined for internal use. In other words, you can not(?) enforce restriction that a public class X can be used only by classes in my application.

This is where I like Eclipse plugin architecture where you can say what packages in my plugin can dependent plugins access during runtime. JSR 277 aimed at bringing this kind of modular features to JDK but it is dead now. Read more about it here, http://neilbartlett.name/blog/2008/12/08/hope-fear-and-project-jigsaw/

Now the only option seems to be OSGi.

Adi