5

According to Explanation on how "Tell, Don't Ask" is considered good OO, I know I should avoid get the state of an object and then decides the actions to take to that object, eg:

Bad:

public class Manager{
    public void scheduleCallback(){
        if(this.systemMonitor.temperature>100){
            this.systemMonitor.soundAlarms();
        }
    }
}

Good:

public class Manager{
    public void scheduleCallback(){
        this.systemMonitor.scheduleCallback();
    }
}

public class SystemMoniter{
    public void scheduleCallback(){
        if(this.temperature>100){
            this.soundAlarms();
        }
    }
}

However, in practice, I found the example above is not similar to real code, instead SystemMoniter may need to call other objects from its parent:

public class Manager{
    private LED led;
    private Speaker speaker;
    private Screen screen;
    public void scheduleCallback(){
        this.systemMonitor.scheduleCallback(LED led,Speaker speaker,Screen screen);
    }
}

public class SystemMoniter{
    public void scheduleCallback(LED led,Speaker speaker,Screen screen){
        if(this.temperature>100){
            led.turnRed();
            speaker.play("alarm.mp3");
            screen.play("alarm.mp4");
        }
    }
}

which I think moving the logic that requires parent objects to Manager and then asks systemMoniter to get the temperature is more simple:

public class Manager{
    private LED led;
    private Speaker speaker;
    private Screen screen;
    public void scheduleCallback(){
        if(this.systemMoniter.temperature>100){
            led.turnRed();
            speaker.play("alarm.mp3");
            screen.play("alarm.mp4");
        }
    }
}

I tried passing a callback to SystemMoniter:

public class SystemMoniter{
    public void scheduleCallback(Runnable runnable){
        if(this.temperature>100){
            runnable.run();
        }
    }
}

public class Manager{
    private LED led;
    private Speaker speaker;
    private Screen screen;
    public void scheduleCallback(){
        this.systemMoniter.scheduleCallback(new Runnable(){
            public void run(){
                led.turnRed();
                speaker.play("alarm.mp3");
                screen.play("alarm.mp4");
            }
        });
    }
}

But I found it is worse because the codes with callback requires me to yo-yo between caller and callee to understand, which is a maintain nightmare.

And sometimes I found "Tell, don't ask" may result in circular dependency:

Tell, don't ask:

public class A{
    public B b;
    public void run(){
        if(this.state){
            //some code in A
            this.b.run();
        }
    }
}
public class B{
    public A a;
    public void run(){
        if(this.state){
            //some code in B
            this.a.run();
        }
    }
}

non Tell, don't ask (move the logic and action to perform to parent):

public class ManagerOfAB(){
    public void run(){
        if(b.state){
          //some code in A
        }
        if(a.state){
          //some code in B
        }
    }
}

which I think the "non Tell, don't ask" version is easier to understand. And real world, for example, there may need a engineer to check the system actively, if the engineer finds the system is not working, some actions may need to take to repair the system, which is a kind of "get state of that object, then decide to do something of that object", right? So I think "get state of that object, then decide to do something of that object" is not always bad.

So my question is, according to the codes above, are "need to call objects in parent object" and "avoid circular dependency" reasons to avoid "Tell, don't ask"?

2
  • 2
    Just as in real life, managers are often redundant ;-) Why don't you move Led, Speaker and Screen into SystemMonitor?
    – Rik D
    Commented Sep 27, 2023 at 8:24
  • 9
    Alternatively, you could consider a more loosely coupled pub/sub design, where Led, Speaker and Screen subscribe to some TemperatureLimitExceededEvent
    – Rik D
    Commented Sep 27, 2023 at 8:35

4 Answers 4

28

Ultimately, you're pushing all the program logic into class Manager. Everything else is only a dumb interface. As the program develops, Manager gets bigger and bigger, until it's an unmaintainable blob.

The OO way is to push knowledge into the appropriate classes. Why does Manager need to know that a temperature > 100 is the alarm level? Push that knowledge into SystemMonitor.

But then, why should SystemMonitor know what sound file to play when there's an alarm? Push that knowledge into another class, and just tell that class to do an over-temperature alarm.

While you're at it, try to come up with more meaningful names for the functions. "scheduleCallback" doesn't actually schedule a callback. And on class SystemMonitor, it probably should be something like "checkTemperature".

1
  • 20
    The last paragraph is important. In fact, I wouldn't be surprised that just trying to name things would result in recognizing a better design. Commented Sep 27, 2023 at 13:32
9

The point is to reduce how much Manager knows. Also, SystemMoniter should only know its things. it shouldn't need to know about Leds, Speakers, or the Screen. And actually, neither should the Manager.

class Manager{
    // maybe Manager should only know a List<CallbackSchedulerInterface>,
    // and not know about SystemMoniter directly?
    SystemMonitor systemMonitor; 
    public void scheduleCallback(){
       systemMonitor.scheduleCallback();
    }
} 

There should be an alarm listener interface

public interface SystemAlarmListener {
   void onSystemAlarmGoesOff();
}

And then the SystemMonitor only knows about the interface

private class AlarmListeners {
    private Set<SystemAlarmListener> alarmListeners;
    public onSystemAlarmGoesOff() {
        for(SystemAlarmListener listener : alarmListeners)
            listener.onSystemAlarmGoesOff();
    }

}
class SystemMoniter {
    AlarmListeners alarms;
    public void scheduleCallback() {
        if(this.temperature>100)
           alarms. onSystemAlarmGoesOff();
    }
}

And all your other things implement that interface

class LED implements SystemAlarmListener {
    public onSystemAlarmGoesOff() {
        led.turnRed();
    }
}
class Speaker implements SystemAlarmListener {
    public onSystemAlarmGoesOff() {
        speaker.play("alarm.mp3");
    }
}
class Screen implements SystemAlarmListener {
    public onSystemAlarmGoesOff() {
        screen.play("alarm.mp4");
    }
}

Note that each class is only told, never asked. Each class manages it's single responsibility, and responsibility is not shared. And, bonus: Note that since everything depends on the tiny SystemAlarmListener, now all of your classes can compile in parallel, which makes compilation MUCH faster (depending on your build system).

0
4

The problem I see here is a good old fashioned coupling problem.

Your 'bad' does too much. Then in 'good' you delegate to something else that also does too much. This is not progress.

public class Manager{
    public void poll(){
        system.checkTempBelow(100, Alarm::klaxon);
    }
}

This is tell don't ask. No questions being asked. Just telling things what do do and not caring what happens afterwards because that's something else's job to worry about.

But this is still doing a lot. It knows when to check, what amount to check, and what to do about it. Yuck. Why so busy?

Because we configured everything here. We don't have to.

public class Manager{
    public void poll(){
        temperatureAlarm.check();
    }
}

There. Now temperatureAlarm knows what temp to care about. We don't. It knows what alarm to sound. We don't. All we know is when to check. We don't know if the alarm is going off. We don't care. That's loosely coupled. God willing this thing doesn't throw anything ever and we can just call check and move on with our lives.

This is still tell don't ask. But ideally it's the end of a long line of telling one thing at a time so all we have know here is when to check. You can allow that by just asking to be handed a configured temperatureAlarm.

The benefit of this information hiding is we are unlikely to need to change. We only know when to call. Some law gets passed that forces a change from Fahrenheit to Celsius, this class doesn't care. Turns out the Klaxon alarm is under copyright, this class doesn't care. Limit the spread of knowledge and the details of those decisions can each be controlled in one place.

Tell don't ask isn't everything. It's a very useful principle to get you to stop turning objects inside out. But there is more to configuring and writing code with an event driven focus than just tell don't ask.

As for circular references, yes STATIC circular references are evil. They allow code changes to circulate ad nauseam. Dynamic circular references through stable abstract interfaces are not so bad. See the observer pattern.

Code changes tend to cause code changes. A good design walls that off by stopping the change when it hits code that doesn’t care about it.

0

So my question is, according to the codes above, are "need to call objects in parent object" and "avoid circular dependency" reasons to avoid "Tell, don't ask"?

Those are implementation details that could be avoided in the case from description implementing command design pattern.

"Tell, don't ask" principle is about optimistic programming, that is program from the premise that everything runs successfully. Implementation details shouldn't be a reason to avoid following a principle.

1
  • Your answer could be improved with additional supporting information. Please edit to add further details, such as citations or documentation, so that others can confirm that your answer is correct. You can find more information on how to write good answers in the help center.
    – Community Bot
    Commented Sep 28, 2023 at 17:17

Not the answer you're looking for? Browse other questions tagged or ask your own question.