Question: Is it better to use getters or to access private members directly?

Question

Is it better to use getters or to access private members directly?

Answers 6
Added at 2016-12-06 13:12
Tags
Question

Which of the following is better? Is it even opinion-based or are there any relevant differences? Can one or the other be favored in some scenarios?

public class MyClass {
    private Integer myField;

    public void setMyField(Integer myField) {
        this.myField = myField;
    }

    public Integer getMyField() {
        return myField;
    }

}

I need a method to check wether something is allowed or not. Please, let's not talk about the sense of this code example. It's just a minimal example.

Implementation 1

public boolean isAllowed() {
    MyEnum.ALLOWED.getInt().equals(getMyField());
}

Implementation 2

public boolean isAllowed() {
    MyEnum.ALLOWED.getInt().equals(myField);
}

Edit: This post does not have an answer in the linked question (see comments to the initial post)

Answers
nr: #1 dodano: 2016-12-06 13:12

In my Software Engineering courses I was told to realize the "principle of secret". Thus, you should always use getter- and setter-routines. This way you can be sure that nobody accesses the member variable by accident.

Strange functions or objects may never see or even change member variables except you explicitly tell them to do so by setter and getters.

nr: #2 dodano: 2016-12-06 13:12

I would recommend using the getter because in certain scenarios, it can have some additional logic (like formatting, checking for nulls and so on). So you may be loosing some logic when using the field itself.

nr: #3 dodano: 2016-12-06 13:12

Due to your attribute being private, you can only securely access it within other class using getter or setter methods. So I would say that the best implementation is the one following the encapsulating principle, i.e., the one using the getter instead of accessing directly. This will prevent data leaks as well.

https://www.tutorialspoint.com/java/java_encapsulation.htm

nr: #4 dodano: 2016-12-06 13:12

Which of the following is better? Is it even opinion-based or are there any relevant differences? Can one or the other be favored in some scenarios?

It is question of good practice I think. The difference is in the readability of the code.

As a general rule, you should avoid indirection if not required. The current instance of MyClass has the information in one of these fields to implement the operation. It doesn't need to hide its internal state to itself.
So in internal, MyClass has no valuable reason to favor the use of the getMyField() over the direct use of the myField field.
The getMyField() accessor is more suitable to be used by clients of the class.
So I think that it is better in any case in your example code :

public boolean isAllowed() {
    MyEnum.ALLOWED.getInt().equals(myField);
}

Edit :
Beyond the readability, here is an example why you have no interest to couple the internal state to a public getter.
Suppose during the development phase you remove from the class the public getMyField() method because not need or not needed any longer for clients of the class, if isAllowed() relies on getMyField() in its implementation, it will be broken and you should replace it by myField.

nr: #5 dodano: 2016-12-06 13:12

To keep a good encapsulation, the first think you need to think is which methods are you going to expose outside your class, if here, for example you are going to use only the is allowed method, I would make public only that method, and define the field in the constructor, if the field is suitable to change then you will need getter/setters but always depends on what do you want to offer from your class. And keep as much encapsulated as you can.

public class MyClass {
    private Integer myField;

    MyClass(Integer myField){
        this.myField = myField;
    }

    //Only this method is offered nobody need to know you use myField to say true/false
    public boolean isAllowed() {
        MyEnum.ALLOWED.equals(myField);
    }

}
nr: #6 dodano: 2016-12-06 20:12

My answer won't be the most informative however it will come from direct experience of dealing with this pattern. When designing an object it is often tempting to directly access member fields rather than rely on accessors. The desire stems from wanting to simplify the object and avoid adding clutter from methods that simple return a value. Taking your example a step further to add context & meaning:

public class MyClassmate {
    private Integer age;

    public MyClassmate(Integer age) {
        this.age = age;
    }

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

    public Integer getAge() {
        return age;
    }

}

Here age is a simple number and it appears unnecessary to add getters/setters around it. If we add the following method you would be tempted to directly access the field since there is no change in behavior:

public Integer calculateScore() {
    if(age > 21) return 100 - getNumberOfIncorrectAnswers();
    //Add 10% before for younger students
    else return (100 - getNumberOfIncorrectAnswers()) + 10;
}

Your object may then grow new features with methods relying on the age field where you continue to use it directly. Later, you might alter the way age is originated and pull the value from across a network. You might not want to incorporate the networking logic in the constructor because it is an expensive operation which should only be triggered as needed. The calculateScore() method could make the network connection and discover the age but then too would all of the other methods that rely on age. But what if calculateScore looked as follows?:

public Integer calculateScore() {
    if(getAge() > 21) return 100 - getNumberOfIncorrectAnswers();
    //Add 10% before for younger students
    else return (100 - getNumberOfIncorrectAnswers()) + 10;
}

You could then enhance the object changing the way it derives age without touching the calculateScore() method. This means your method follows Open Closed Principle (OCP). It is open for enhancement but closed to change, or you didn't have to change the method source in order to change where it gets the age.

Depending on the complexity of your app and object model there may still be times when encapsulated access is overkill but even in these scenarios it is good to understand the tradeoffs of direct field access and these more simplistic scenarios are mostly rare.

In general you should understand that the need for encapsulation is almost never immediate. It appears over time as the object grows and if the object is not setup with encapsulation from its onset it is more expensive to phase it in. That's what makes this approach so difficult to appreciate. It takes experience (i.e. making the typical oversimplification and suffering several times over several years) to feel why encapsulation is necessary. It is not something you can eyeball and detect.

That said, this used to be a much bigger problem than it is today when IDEs were not as full featured. Today you can use the built in encapsulate fields refactoring in certain IDEs like IntelliJ to introduce the pattern as you need it. Even with modern IDEs it is still favorable to practice encapsulation from the onset.

Source Show
◀ Wstecz