Search code examples
javaoopcoding-style

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


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)


Solution

  • 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.