Is it bad practice to have my getter method change the stored value?

In my opinion, unless you are doing lazy-loading (which you are not in that case), getters should not change the value. So I would either:

Put the change in the setter

public void setMyValue(String value) {
    if(value == null || value.isEmpty()){
        this.myValue = "N/A";
    } else {
        this.myValue = value;
    }
}

Or make the getter return a default value if value not set properly:

public String getMyValue() {
    if(this.myvalue == null || this.myvalue.isEmpty()){
        return "N/A";
    }    
    return this.myValue;
}

In the case of lazy-loading, where I would say that changing your members in a getter is fine, you would do something like:

public String getMyValue() {
    if (this.myvalue == null) {
        this.myvalue = loadMyValue();
    }    
    return this.myValue;
}

I think it is actually quite a bad practice if your getter methods change the internal state of the object.

To achieve the same I would suggest just returning the "N/A".

  • Generally speaking this internal field might be used in other places (internally) for which you don't need to use the getter method. So in the end, the call to foo.getMyValue() could actually change the behaviour of foo.

Alternatively, the translation from null to "N/A" could be done in the setter, i.e. the internal value could be set to "N/A" if null is passed.


A general remark:
I would only add states such as "N/A" if they are expected by some API or other instance relying on your code. If that is not the case you should rely on the standard null types that are available to you in your programming language.