Take the 2-minute tour ×
Programmers Stack Exchange is a question and answer site for professional programmers interested in conceptual questions about software development. It's 100% free, no registration required.

This blogpost was posted on Hacker News with several upvotes. Coming from C++, most of these examples seem to go against what I've been taught.

Such as example #2:

Bad:

def check_for_overheating(system_monitor)
  if system_monitor.temperature > 100
    system_monitor.sound_alarms
  end
end

versus good:

system_monitor.check_for_overheating

class SystemMonitor
  def check_for_overheating
    if temperature > 100
      sound_alarms
    end
  end
end

The advice in C++ is that you should prefer free functions instead of member functions as they increase encapsulation. Both of these are identical semantically, so why prefer the choice that has access to more state?

Example 4:

Bad:

def street_name(user)
  if user.address
    user.address.street_name
  else
    'No street name on file'
  end
end

versus good:

def street_name(user)
  user.address.street_name
end

class User
  def address
    @address || NullAddress.new
  end
end

class NullAddress
  def street_name
    'No street name on file'
  end
end

Why is it the responsibility of User to format an unrelated error string? What if I want to do something besides print 'No street name on file' if it has no street? What if the street is named the same thing?


Could someone enlighten me on the "Tell, Don't Ask" advantages and rationale? I am not looking for which is better, but instead trying to understand the author's viewpoint.

share|improve this question
    
Code examples might be Ruby and not Python, I dunno. –  Pubby Jul 20 '12 at 0:36
1  
Whoever came up with this maybe took the repeal of “don’t ask don’t tell” out of context … –  Konrad Rudolph Jul 20 '12 at 7:00
    
I always wonder if something like the first example isn't rather a violation of SRP? –  stijn Jul 20 '12 at 7:33
    
You may read that: pragprog.com/articles/tell-dont-ask –  Mik378 Nov 15 '12 at 1:12
    
Ruby. @ is shorthand for instance and Python ends its blocks implicitly with whitespace. –  Erik Reppen Feb 27 '13 at 6:01
add comment

6 Answers

up vote 36 down vote accepted

Asking the object about its state, and then calling methods on that object based on decisions made outside of the object, means that the object is now a leaky abstraction; some of its behavior is located outside of the object, and internal state is exposed (perhaps unnecessarily) to the outside world.

You should endeavor to tell objects what you want them to do; do not ask them questions about their state, make a decision, and then tell them what to do.

The problem is that, as the caller, you should not be making decisions based on the state of the called object that result in you then changing the state of the object. The logic you are implementing is probably the called object’s responsibility, not yours. For you to make decisions outside the object violates its encapsulation.

Sure, you may say, that’s obvious. I’d never write code like that. Still, it’s very easy to get lulled into examining some referenced object and then calling different methods based on the results. But that may not be the best way to go about doing it. Tell the object what you want. Let it figure out how to do it. Think declaratively instead of procedurally!

It is easier to stay out of this trap if you start by designing classes based on their responsibilities, you can then progress naturally to specifying commands that the class may execute, as opposed to queries that inform you as to the state of the object.

http://pragprog.com/articles/tell-dont-ask

share|improve this answer
1  
The example text disallows many things which are clearly good practice. –  DeadMG Jul 20 '12 at 1:38
8  
@DeadMG it does what you say only to those slavishly follow it, who blindly ignore "pragmatic" in the site name and key thought of site authors that has been clearly stated in their key book: "there is no such thing as a best solution..." –  gnat Jul 20 '12 at 5:01
    
Never read the book. Nor would I wish to. I only read the example text, which is completely fair. –  DeadMG Jul 20 '12 at 5:46
2  
@DeadMG no worries. Now that you know key point that puts this example (and any other one from pragprog for that matter) in the intended context ("no such thing as a best solution..."), it's OK not to read the book –  gnat Jul 20 '12 at 13:10
    
I'm still not sure what Tell, Don't Ask is supposed to spell out for you without context but this is really good OOP advice. –  Erik Reppen Feb 27 '13 at 6:10
add comment

Generally, the piece suggests that you should not expose member state for others to reason about, if you could reason about it yourself.

However, what's not clearly stated is that this law falls into very obvious limits when the reasoning is way over the responsibility of a specific class. For example, every class whose job is to hold some value or provide some value- especially generic ones, or where the class provides behaviour that must be extended.

For example, if the system provides the temperature as a query, then tomorrow, the client can check_for_underheating without having to change SystemMonitor. This is not the case when the SystemMonitor implements check_for_overheating itself. Thus, a SystemMonitor class whose job is to raise an alarm when the temp is too high does follow this- but a SystemMonitor class whose job is to allow another piece of code to read the temperature so that it can control, say, TurboBoost or something like that, should not.

Also note that the second example pointlessly uses the Null Object Anti-pattern.

share|improve this answer
9  
“Null object” isn’t what I’d call an anti-pattern, so I wonder what’s your reason for doing so? –  Konrad Rudolph Jul 20 '12 at 7:04
1  
Pretty sure that nobody has any methods which are specified as "Does nothing". That makes calling them kinda pointless. That means that any object implementing Null Object breaks LSP, at the very least, and describes itself as implementing operations that it, in fact, does not. The user expects a value back. Their program's correctness depends on it. You simply bring more problems by pretending that it is a value when it is not. Have you ever tried to debug silently failing methods? It is impossible and nobody should ever have to suffer through that. –  DeadMG Jul 20 '12 at 7:39
2  
I’d argue that this entirely depends on the problem domain. –  Konrad Rudolph Jul 20 '12 at 8:27
2  
@DeadMG I agree that the above example is a bad usage of the Null Object pattern, but there is a merit to using it. A few times I've used a 'no-op' implementation of some interface or other to avoid null-checking or having true 'null' permeating the system. –  Max Jul 20 '12 at 10:20
3  
Not sure I see your point with "the client can check_for_underheating without having to change SystemMonitor". How is the client different from SystemMonitor at that point? Aren't you now dissipating your monitoring logic across multiple classes? I also don't see the problem with a monitor class that provides sensor information to other classes while reserving alarm functions to itself. The boost controller should be controlling boost w/o having to worry about raising an alarm if the temperature gets too high. –  TMN Jul 20 '12 at 13:26
add comment

The real issue with your overheating example is that the rules for what qualifies as overheating are not easily varied for different systems. Suppose System A is as you have it (temp>100 is overheating) but System B is more delicate (temp>93 is overheating). Do you change your control function to check the type of system, and then apply the correct value?

if (system is a System_A and system_monitor.temp >100)
  system_monitor.sound_alarms
else if (system is a System_B and system_monitor.temp > 93)
  system_monitor.sound_alarms
end

Or do you have each type of system define its heating capacity?

EDIT:

system.check_for_overheating

class SystemA : System
  def check_for_overheating
    if temperature > 100
      sound_alarms
    end
  end
end

class SystemB : System
  def check_for_overheating
    if temperature > 93
      sound_alarms
    end
  end
end

The former way makes your controlling function get ugly as you start dealing with more systems. The latter lets the control function be stable as time goes on.

share|improve this answer
    
Why not have each system register with the monitor. During registration they can indicate when overheating occurs. –  Loki Astari Jul 20 '12 at 19:47
    
@LokiAstari - You could, but then you could run into a new system that's also sensitive to humidity or atmospheric pressure. The principle is to abstract out what varies--in this case it is the susceptibility to overheating –  Matthew Flynn Jul 20 '12 at 20:59
    
This is exactly why you should have a tell model. You tell the system the current conditions and it informs you if it is outside normal working conditions. Thus way you never need to modify the SystemMoniter. That's encapsulation for you. –  Loki Astari Jul 20 '12 at 23:40
    
@LokiAstari - I think we're talking at cross purposes here--I was really looking at creating different systems, rather than different monitors. The thing is, the system should know when it is in a state that raises an alarm, as opposed to some outside controller function. SystemA should have its criteria, SystemB should have its own. The controller should just be able to ask (at regular intervals) whether the system is OK or not. –  Matthew Flynn Jul 21 '12 at 2:54
add comment

First off, I feel I must take exception to your characterization of the examples as "bad" and "good". The article uses the terms "Not so good" and "Better", I think those terms were chosen for a reason: these are guidelines, and depending on circumstances the "Not so good" approach may be appropriate, or indeed the only solution.

When given a choice, you should give preference to including any functionality that relies solely upon the class in the class instead of outside it -- the reason is because of encapsulation, and the fact that it makes it easier to evolve the class over time. The class also does a better job of advertising it's capabilities than a bunch of free functions.

Sometimes you have to tell, because the decision relies upon something outside of the class or because it is simply something you don't want most users of the class to do. Sometimes you want to tell, because the behavior is counter intuitive for the class, and you don't want to confuse most users of the class.

For example, you complain about the street address returning an error message, it isn't, what it is doing is providing a default value. But sometimes a default value isn't appropriate. If this was State or City, you might want a default when assigning an a record to a salesman or a survey taker, so that all of the unknowns go to a specific person. On the other hand, if you were printing envelopes, you might prefer an exception or guard that keeps you from wasting paper on letters that can't be delivered.

So there can be cases where "Not so good" is the way to go, but generally, "Better" is, well, better.

share|improve this answer
add comment

In addition to the other good answers about "tell, don't ask", some commentary on your specific examples that might help:

The advice in C++ is that you should prefer free functions instead of member functions as they increase encapsulation. Both of these are identical semantically, so why prefer the choice that has access to more state?

That choice does not have access to more state. They both use the same amount of state to do their jobs, but the 'bad' example requires the class state to be public in order to do its work. Further, the behavior of that class in the 'bad' example is spread out to the free function, making it harder to find and more difficult to refactor.

Why is it the responsibility of User to format an unrelated error string? What if I want to do something besides print 'No street name on file' if it has no street? What if the street is named the same thing?

Why is it the responsibility of 'street_name' to do both 'get street name' and 'provide error message'? At least in the 'good' version, each piece has one responsibility. Still, it's not a great example.

share|improve this answer
2  
That's not true. You presume that checking for overheating is the only sane thing to do with the temperature. What if the class is intended to be one of a number of temperature monitors, and a system must take different action depending on many of their results, for example? When this behaviour can be limited to pre-defined behaviour of a single instance, then sure. Else, it obviously can't apply. –  DeadMG Jul 20 '12 at 1:47
    
Sure, or if the thermostat and the alarms existed in different classes (as they likely should). –  Telastyn Jul 20 '12 at 1:53
1  
@DeadMG: The general advice is to make things private/protected until you need access to them. While this particular example is meh, that doesn't dispute the standard practice. –  Guvante Jul 20 '12 at 17:50
add comment

I believe this is more true when writing high-level object, but less true when going down to the deeper level e.g. class library as it's impossible to write every single method to satisfy all class consumers.

For example #2, I think it is over-simplified. If we were actually going to implement this, the SystemMonitor would end up having the code for low level hardware access and logic for high level abstraction embedded in the same class. Unfortunately, if we are trying to separate that into two classes, we would violate the "Tell, Don't ask" itself.

The example #4 is more or less the same -- it's embedding UI logic into data tier. Now if we are going to fix what user wants to see in case of no address, we have to fix the object in data tier, and what if two projects using this same object but need to use different text for null address?

I agree that if we can implement "Tell, Don't ask" for everything, it would be very useful -- I myself would be happy if I can just tell rather than ask (and do it myself) in real life! However, as same as in the real life, the feasibility of the solution is very limited to high level classes.

share|improve this answer
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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