Is it considered as bad practice to update observers in fileds' setters?

Question

Let's say I have an object Subject which consists of a list of its observers and an int field:

package example.template.pattern.observer;

import java.util.ArrayList;
import java.util.List;

public class Subject {

    private List<Observer> observers = new ArrayList<Observer>();
    private int state;

    public int getState() {
        return state;
    }

    public void setState(int state) {
        this.state = state;
    }

    public void attach(Observer observer) {
        observers.add(observer);
    }

    public void notifyAllObservers() {
        for (Observer observer : observers) {
            observer.update();
        }
    }
}

I would like to notify all of them after every 'set' operation of state's field. The most common code I know for performing such an operation is something like this:

Subject subject = new Subject();
subject.add(newObserver);
subject.setState(newState);
subject.notifyAllObservers();

But because I want an update every time I set a new value to the state I changed a code a little.

Changed notifyAllObservers() access modifier to private:

private void notifyAllObservers() { ... code }

And added a new extra line to the state's setter:

public void setState(int state) {
    this.state = state;
    notifyAllObservers();
}

Is the code above considered as a bad practice?

Answer 1

Why wouldn't this be fine?

In my opinion, this is actually considered a good practice. You will have a very high chance of forgetting to call notifyAllObservers after maybe a month or so. Or even worse, someone else might use your code and don't know that they should call notifyAllObservers after setting the state. If this happens, your code might not work as expected. That's why you should put notifyAllObservera in the setState method. This way, you and other people don't need to worry about it.