0

Hi I am using the MVVM pattern in an android project, and for the ViewModel have code like this:

public class LoginViewModel extends ViewModel {

    public MutableLiveData<User> user = new MutableLiveData<>();
    public MutableLiveData<String> email = new MutableLiveData<>();
    public MutableLiveData<String> password = new MutableLiveData<>();
    public MutableLiveData<Boolean> emailError = new MutableLiveData<>();
    public MutableLiveData<Boolean> register = new MutableLiveData<>();
    private LoginRespository loginRespository = new LoginRespository();
    private MutableLiveData<Boolean> enable = new MutableLiveData<>();

    public LoginViewModel() {
    }

    public void login() {
        ...
    }

    public void startRegister() {
        ...
    }

    private String getEmailValue() {
        if (email.getValue() == null) {
            email.setValue("");
        }

        return email.getValue();
    }
....
}

In the office, we have a discussion about that use the set inside the get is a bad practice, but I think that this is not a bad practice because java allows nulls, and I want not to get a null value when calling the getEmailValue() I think that the concept of encapsulation is for these cases.

The question is if really this can be a bad practice, or not?

Thanks

Tlaloc-ES
  • 4,825
  • 7
  • 38
  • 84

1 Answers1

1

Yes, this is bad practice; your getters should not modify any state (themselves or by calling setters), just inspect and expose it.

The solution to your specific problem is to do one of two things:

  1. Let your getter return an empty string instead of null, without modifying the stored value.

    private String getEmailValue() {
    String emailValue = email.getValue();
    return emailValue != null ? emailValue : "";
}

  2. Let your setter replace an incoming null value with the empty string.

    private String setEmailValue(String email) {
    email.setValue(email != null ? email : "");
}

This way, you can be sure that when you call getEmailValue() you will always get a non-null String, but you never modify the object from your setter.

Tomas Aschan
  • 58,548
  • 56
  • 243
  • 402
  • But this get is not for the MutableLiveData, if not for value of the mutable live data, this and it is possible that i need call thet get before the set then for that case, I will get a null value. – Tlaloc-ES Feb 12 '19 at 13:03
  • anyway the first option seems good to keep the concetp that the get does not modify the value – Tlaloc-ES Feb 12 '19 at 13:04
  • @Tlaloc-ES: If it's important to ensure that the underlying `MutableLiveData` never contains a null value, then use option 2. On the other hand, there's nothing that prevents you from doing _both_. A third option, that you can use in conjunction with either of the ones listed above, is to initialize the `MutableLiveData` instance with a default value of `""` rather than `null`, to ensure that if you try to get the value before setting it, you still don't get a null value. – Tomas Aschan Feb 12 '19 at 13:11