10

Consider the following class:

public class Person 
{
private Integer age;

// Standard Accessors
public Integer getAge() {
    return age;
}

public void setAge(Integer age) {
    this.age = age;
}

public String getAgeAsTextString()
{
    if (this.age == 20)
    {
        return "Twenty";
    }
    return "Unknown";
}
}

I just have 1 Integer, and 2 accessors. If I want to create a utility method that returns the objects' state as a String, is it best practice to refer to the class variable as this.age, or should I be using getAge()?

Is there a best practice or is it down to developer discression?

11 Answers 11

8

I'd say its down to developer discretion.

I slightly prefer using the getter method. And if you have a class hierarchy, it's good to expose internal state via protected getters.

7

Depends. For small fast classes where accessors do nothing, I would say use the field(s) directly. For larger classes where accessors and setter might have side effects I would say use accessors.

On the Android platform the Android documentation seems to agree. They even go as far as recommending Package level protection in certain cases. See the sections titled "Avoid Internal Getters/Setters" and "Use Package Scope with Inner Classes" in http://developer.android.com/guide/practices/design/performance.html.

It all depends on the environment, the size of the code, and if there are agreed "standards" for code maintenance reasons. Just know that there is some tiny performance difference but in the end it's up to the developer/team.

0
3

It's a good practice because you can never be sure how you might want age to change at a later date. It might need to have some kind of validation or something else and it's easier to change it in one method than everywhere the field is used (and it's a lot easier to miss one place where its used and thus have some difficulty tracking down a potential error).

1
  • 1
    Usually you have validation on setter rather than the getter and the simplest way to make sure you change all references is to either; make the field private (so you don't have dependancies in other classes), make the change which would break the code (and let the compile change the usages), or let your IDE find all usages. Dec 3, 2010 at 15:47
3

If you class would be final, I'd access the fields directly. Trust in your refactoring capabilities. If you want to change it later, you always can do it.

1
  • 1
    I second that. Even if you have to refactor your code later on, that shouldn't be much of a problem when using a potent IDE. Dec 3, 2010 at 17:24
2

For the love of god, don't mess your code up with oxymoronic "self encapsulation". The class is your module. Make your class simple.

Also, design your class' interface in terms of behaviour instead of data. Ditch those get and set methods.

2

I like using getters and setters even for simple classes because it creates "Hooks" in your code, making it easy to extend. So, if you want to create an audit entry each time a user changes a certain object property - it is easy to do. Then, when I get to the end of my project, I'll take a moment and delete the getters and setters I didn't use. I find this effective because most IDE's will generate the accessors for you so there isn't much time wasted...

I see people posting, "If you want to change it later, you always can do it.". This is not always trivial in large projects, when you are directly accessing your object properties all throughout your code.

1

Often people go with this.age in a simple class, but consider the benefits of using accessors when you already have them. If you want to introduce one day, say, logging when someone gets the age, you'd have to put it in every single place you call this.age. Or instead, you can just put it in the accessor. Code style isn't always fixed, but usually there's a good reason if you look for it.

1

One thing to remember. When using something like Hibernate, using the accessors is generally required! Have gotten bitten by this one a few times. Hibernate uses byte code generation to do lazy initialization. if you access the member variable directly, you will skip the lazy loading and get null values. So, my personal best practice is to use the accessor. Like all best practices, of course, there are times to ignore it, but there you go.

1

A few reasons why accessors "can" be a better option:

  1. You are using a mocking framework to test your code and want to be able to BOTH change the value returned from a getter, AND assert that it was called.
  2. You want to expose most (if not all) of your public classes as interfaces (e.g. you want to be able to create dynamic proxies etc). This may include exposing accessors in the interface (the caller doesn't need to know it's simply retrieving a local property)
  3. You are developing a library for public consumption, and a "simple refactor" is not appropriate as it may (will) cause compile time errors in the end user's application. Accessors can be deprecated appropriately without affecting the behavior for users of the earlier library.
  4. You want to reserve the right to change the behavior of the accessor at a later point, or in a derived class without removing it entirely (see previous point).
  5. Many dependency injection frameworks will expect a "bean" to conform to the JavaBeans spec and thus have accessors for all properties.
0

IMHO: I follow the You-Aint-Gonna-Need-It priniciple of keeping things simple. Many developers are concerned that one day, may be you might need a getter or setter, when actually this never happens.

You should do what you believe is clearest, esp to those who have to maintain your code.

I would suggest it is good practice to use int, unless you need the value to be null or an object. If you need the value to be potentally null, you should test for it.

i.e. either

int age = 10;
if (this.age == 20) // cannot be null.

OR

Integer age = 10;
if (this.age != null && this.age == 20) // age can be null

Note

Integer age;
if (this.age == 20) // throws an NPE.
8
  • 2
    -1 Irrelevant and off-topic. The question isn't about int vs Integer. Also, IMO there are parentheses missing from your sample code. Dec 3, 2010 at 15:36
  • @Erick, IMHO he has a bug in his code. He was asking about best practice, but he should also be concerned about correctness first. Where would you put the parenthesis? Dec 3, 2010 at 15:41
  • apart from possible bugs in this answer, here's my opinion about YAGNI: programmers.stackexchange.com/questions/14856/… Dec 3, 2010 at 16:31
  • if ((this.age != null) && (this.age == 20)) Dec 5, 2010 at 15:11
  • @Erick, Thank you for clarifying. I am inclided to only place parentheses where they are needed by the compiler. Your answer would be clearer to those not comfortable with operator precidence who have a C background. But in Java you can't place parenteses in any other combination and have it compile. i.e. its the only possibility so its redundant. Dec 6, 2010 at 10:24
0

While accessors to add lines of code to your classes, they really add value when refactoring code. You need to notify another class that a property has changed? BAM! there's an event and trigger it within the accessor code.

With just making the member variable accessible, you'd have to either create the accessor when you needed to create the event, or remember to append the logic to notify the object every time an object outside your class changed the member variable.

I think accessors are great for their flexibility to complexity ratios. Plus, some frameworks, like hibernate requires them. Also, languages like .net have code structures like attributes that can't be applied to member variables (aka WCF DataMember).

hope that helps!

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.