Is it a bad practice for my getter method to change stored value?

Question

Is it a bad practice to change my getter method as version 2 in my class.

Version 1:

public String getMyValue(){ return this.myValue } 

Version 2:

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

Answer 1

I think this is actually pretty bad practice if your getter methods change the internal state of an object.

To achieve this, I would suggest simply returning "N/A" .

• In general, this internal field can be used in other places (inside), for which you do not need to use the getter method. So in the end, calling foo.getMyValue() can really change the behavior of foo .

Alternatively, a translation from null to "N/A" can be done in the setter , that is, the internal value can be set to "N/A" if null passed.

---

General comment:
I would only add states like "N/A" if they are expected by some API or other instance relying on your code. If this is not the case, you should rely on the standard types of zeros available to you in your programming language.

Answer 2

In my opinion, if you are not doing lazy-loading (which you are not in this case), getters should not change the value. Therefore, I would:

Put the change in the installer

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

Or make getter return the default value if the value is 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 getter is ok, you would do something like:

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

Answer 3

No. Here you do two things. Getting and setting.

Answer 4

Yes. This is bad practice.

What for?

When a value is set (in the constructor or installation method), it must be checked, and not when the retrieval method is called. Creating a private validate* method for this is also a good idea.

 private boolean validateThisValue(String a) { return this.myValue != null && !this.myValue.isEmpty(); } public void setThisValue(String a) { if (validateThisValue(a)) { this.myValue = a; } else { // do something else // in this example will be this.myValue = "N/A"; } } 

And in the receiving method, never change the state of an object. I worked on some projects, and the recipient often needs to be const : "this method cannot change the internal state."

At least if you do not want to complicate the situation, in the receiving method you should return "N/A" and not change the internal state and set myValue to "N/A" .

Answer 5

I usually define a specific getter .

Never change the original getter :

  public String getMyValue(){ return this.myValue } 

And create a specific getter :

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

Answer 6

I think it's better to initialize this.myValue = "N/A" . And subsequent calls to setMyValue should change this.myValue to suit your business conditions.
getMyValue should not in any way modify this.myValue . If your needs return a specific value, you should return that value (for example, "N / A"), and not modify this.myValue . Getters must not change the value of a member.

Answer 7

I'd rather change the setter method, so if it is null or empty, this attribute is assigned N/A Thus, if you use the attribute in other methods inside the class (vg toString() ), you will have the specified value.

Alternatively, modify the setter method to throw an exception when the given value is incorrect, so the programmer needs to improve its handling before setting the value.

Other than that, this is normal.

Answer 8

I feel that this is bad practice until and until you explain the reason why it is so necessary that you modify the object inside the getter method and not execute it inside the setter method.
Do you think this is impossible to do for some reason? Could you clarify?

Answer 9

Do whatever you want. After all, getters and setters are another public method. You can use any other names.

But if you use frameworks such as Spring , you must use these standard names, and you should never enter your own codes inside them.

Answer 10

The installer may modify as part of the check, but the receiver must return a value and allow the caller to complete the check. If you do , then how to document it.

Answer 11

It really depends on the contract you want to apply with the get () method. In accordance with standard conventions, the caller must make sure that the preconditions are met (which means that validation in the setter method is often a bad design) and the caller (I don’t know if this English term is correct for what, i.e. the caller) , ensures that the conditions of the message are met.

If you define your contract so that the get () method does not allow you to modify the object, then you are violating your own contract. Think about implementing a type method

 public isValid() { return (this.myvalue == null || this.myvalue.isEmpty()); } 

The advantage of this approach is that you do not need to check that the return of your get () is "N / A" or something else. You can also call this before calling set () to verify that you are not inserting illegal values ​​into your object.

If you want to set the default value, you must do this during initialization.

Answer 12

absolutely yes, this is a bad mark.

Imagine that you are communicating over a network with a third party (remote, COM, ...), this will increase the time it takes to go there, and then hit the performance of the application.

Answer 13

State changes in getters must be dangling violations. This means that client code must be careful about the order in which it refers to getters and setters, and for this it must have knowledge of the implementation. You should be able to call getters in any order and get the same results. A related problem occurs when the setter changes the input value depending on the current state of the object.

Answer 14

For this purpose you can use some value holder. Like an optional class in the guava library.