14

According to Explanation on how "Tell, Don't Ask" is considered good OO, I know the following is bad:

if(a.isX){
    a.doY();
}

public class A{
    public boolean isX;
    public void doY(){
    }
}

which I should modify it to

a.doYWhenX();

public class A{
    private boolean isX;
    public void doYWhenX(){
        if(this.isX){
            this.doY();
        }
    }
    public void doY(){
    }
}

to obey "tell, don't ask" rule. But what if the situation that needs 2 objects? eg:

if(a.isX && b.isP){
    a.doY();
    b.doQ();
}

public class A{
    public boolean isX;
    public void doY(){
    }
}

public class B{
    public boolean isP;
    public void doQ(){
    }
}

How should I refactor it to obey "Tell, don't ask" rule? I tried:

a.doYWhenX(b);

public class A{
    private boolean isX;
    public void doYWhenX(b){
        if(this.isX && b.isP){
            this.doY();
            b.doQ();
        }
    }
    public void doY(){
    }
}

public class B{
    public boolean isP;
    public void doQ(){
    }
}

but A is still asking the state of B, which is not "pure tell,don't ask".

Note: In real code, A and B are some components (eg:Timer, Notice board) which the isX and doY() is not easy to move them into 2 separate classes, and A and B are not easy to combine into one class because some other classes may just use A individually instead of B, for example (which simulates an Page based app, Page1 uses both A and B, but Page2 uses A only):

public static void main(String args[]){
    this.a=new A();
    this.b=new B();
    Page1 page1=new Page1(this.a,this.b);
    this.view.addSubview(page1);
    Page2 page2=new Page1(this.a);
    this.view.addSubview(page2);
}

public class Page1 extends View{
    private A a;
    private B b;
    public Page1(A a,B b){
        if(a.isX && b.isP){
            a.doY();
            b.doQ();
        }
    }
}

public class Page2 extends View{
    private A a;
    public Page2(A a){
        this.a=a;
    }
    public void onButtonPressed(){
        a.doX();
    }
}
9
  • 9
    It is a principle, not a rule. You do it to the extent that you can, because it is generally a good idea, but deviating from it is not necessarily a problem, only a reason to think about it (a "code smell"). Maybe the objects belong together (maybe there should be a "higher level" object containing both), maybe not. Commented Feb 26 at 8:36
  • 3
    I think the "tell, don't ask" involves code where the same class has control over both checking the condition and performing the action. That's not true in your A/B example any longer.
    – chepner
    Commented Feb 26 at 18:47
  • I.e., consider the difference between if(a.isX && b.isP){ a.doY(); b.doQ(); } and a.doYWhenX(); b.doQWhenP(). You can't do "tell, don't ask" without somebody coordinating activity among unrelated classes.
    – chepner
    Commented Feb 26 at 18:55
  • 4
    Is this a concrete problem? Or are you thinking of theoretical examples that might pose a difficulty in parsing this guideline? Because this very much looks like a contrived example whereby the condition is only met when two otherwise unrelated objects match individual conditions which then is supposed to trigger an action in both objects. More likely than not, this is an inefficient object design to begin with, regardless of how it fits with "tell don't ask". I suspect you're either dealing with badly designed code or are working based off of an unfounded theoretical thought exercise.
    – Flater
    Commented Feb 26 at 22:22
  • 1
    @FumbleFingers You seem to have subtly redefined a.isX as a.issubclass(X) (or in Java, a instanceof X), which isn't indicated in the original question — the a.isX and b.isP booleans represent some condition within the respective objects being true, but there's no indication that those checks are questions of class hierarchy/membership.
    – FeRD
    Commented Feb 28 at 2:08

5 Answers 5

28

You wrote

I know the following is bad

and already here is a misconception: thinking religiously in terms of good and bad about this. My interpretation of Tell-Don't-Ask is: when you see a code snippet like

if(a.isX){
    a.doY();
}

you may consider refactoring it to a.doYWhenX(); - or not. But before you decide about this refactoring, also check

  • whether doYWhenX() is a useful abstraction in your context

  • whether doYWhenX() has some reuse potential

  • whether it helps to make a.isX (or a.isX()) private

  • whether it makes the using code more readable

  • is it worth the hassle?

So my recommendation is to change your mindset about the "Tell-Don't-Ask principle" - it is a rough guideline, a rule-of-thumb design heuristic, nothing more.

Now apply this to your case with two objects:

if(a.isX && b.isP){
    a.doY();
    b.doQ();
}

Whether it it more suitable to refactor this

  • to a.doYQWhenXP(b); or

  • to b.doYQWhenXP(a); or

  • to myService.doYQWhenXP(a,b), or

  • leave the code as it is

depends heavily on the context: which variant in your real world context creates highest readability, best reusability, most sensible abstraction, or best encapsulation. Meaningless names like doX or isP alone are not suitable for making such a decision.

3
  • 13
    Agreed. The new name having "when X" in it means that code outside the class knows the rules for when Y-ing is allowed. Bank accounts don't have "WithdrawWhenBalanceisPositive" or "WithdrawWhenNotPastOverdraftLimit" they just have Withdraw. This isn't a mechanical renaming principle, it's the idea of moving the knowledge about when Ying is allowed into the inside of the class that does Y. Especially when the knowledge relates to a member variable of that same class. Commented Feb 26 at 16:12
  • 4
    @KateGregory: I don't disagree, still I think we should not overinterpret the artificial, almost meaningless names the OP used for this example. I think the OP chose doYWhenX to make it clear there is a difference to just doY. In real code, they would probably choose more meaningful names.
    – Doc Brown
    Commented Feb 26 at 16:15
  • 1
    @KateGregory: Besides the code structure, moving the check into the action also has important implications for TOCTOU
    – Ben Voigt
    Commented Feb 27 at 19:19
14

Tell don't ask is a principle aiming at guidance for effectively encapsulating internal details of objects. It therewith:

  • encourages better abstraction, by placing inside the object the logic that belongs to it;
  • facilitates the principle of least knowledge, by avoiding that objects need to rely unnecessarily on internal details of their neighbours.

It is however not an absolute principle and needs to be balanced against sound separation of concerns and single responsibility. In particular, when two objects of different classes are involved, it is vital not to spill unrelated concerns/responsibility into the one or the other object.

In some cases, the solution can be a third object that encapsulates the interaction (e.g. saving accounts and stock accounts could interact via a "transaction" object) that hides the details to the outside world with a tell don't ask but still internally interact with each involved object without forcing them to deal with matters not of their concern.

5
  • 15
    "In some cases, the solution can be a third object that encapsulates the interaction" – This is so often overlooked. Sometimes when you have trouble deciding which existing object to put some behavior on, the solution is: none of them. Listen to your design: if it looks like regardless of which of the two objects you put the behavior on, you'll get in trouble, the design is telling you that it doesn't belong on either of them. Commented Feb 26 at 13:25
  • 3
    That really can't be stressed enough, @JörgWMittag. A lot of design issues can be solved with a separate class that coordinates logic between other classes. Commented Feb 26 at 15:45
  • 4
    @GregBurghardt And almost as many can be created. Truly, this is a versatile technique.
    – wizzwizz4
    Commented Feb 26 at 23:21
  • 7
    All problems in computer science can be solved by another level of indirection, except for the problem of too many layers of indirection. – David J. Wheeler
    – jmoreno
    Commented Feb 26 at 23:58
  • 2
    @jmoreno aren't most of the problems in computer science solved by turning it off and on again? ;-)
    – Christophe
    Commented Feb 27 at 2:54
5

As others have pointed out, "tell, don't ask" is a principle not a rule. It leads your code in a direction which has some good properties, but has other quirks that sometimes you wish to avoid.

The easiest way I know of to work with such a principle is to look at the consequences. The big consequence here is a coupling of algorithm and state. And, the argument made by "tell, don't ask" is that this is a good and desirable thing. It's encapsulation. However, we can't blindly encapsulate everything.

The "tell, don't ask" can make a lot of sense when you can definitively say that there is exactly one correct way to handle any given conditional behavior. Consider the example in the linked question with users and addresses, if you can unequivocally say that claiming a user's address is 'No street name on file' is the only correct way to handle case of asking for a non-existent user's address, and no algorithm could possibly ever want another implementation, then this pattern is great. It encapsulates a behavior that is really closely tied to the object.

... nevermind that that string is probably not going to pass your validation algorithm that looks for a valid street address. So we'll have to handle that case specially. And if this were an email, rather than a street address, you'd probably have to deal with a bunch of latent bugs that manifest in your email server logs where they point out that you keep sending emails to invalid addresses.

There are times where the behavior is more tied to what you want to do with the object than to the object itself. In these cases, it makes more sense to let the algorithm query state, and make its decisions. This is especially true if your objects are part of a library and users are permitted to write algorithms that use said library.

Conditional logic is something that I think more-often-than-not is tied to an algorithm. This is especially obvious after two or more algorithms want to do different things. personally I would scream if I saw a A class (using your example now) which looked like

class A
{
    public void doX();
    public void doY();
    public void doZ();
    public void doXwhenA();
    public void doXwhenB();
    public void doYwhenA();
    public void doYwhenC();
    public void doZwhenAandBbutNotC();
    public void doZwhenPhaseOfMoonIsFull();
    public void doZifAOtherwiseDoX();
}

In these cases, sure, encapsulation is a good ideal. But you encapsulated the wrong thing. You encapsulated something that really didn't belong inside A's bubble in the first place. And now it is threating to pop A's bubble outright.

1
  • Is Phase a public property of Moon? Commented Feb 28 at 14:26
1
if(a.isX){
   a.doY();
}

This is not a violation of "Tell, don't ask". Directly accessing the data-member isX is not nice, but that can be easily remedied with a accessor:

public class A{
    private boolean X;
    public boolean isX(){
        return this.isX;
    }
    public void doY(){
    }
}

if( a.isX() ){
    a.doY();
}

Asking an object for (a portion of) its state is not a violation of "Tell, don't ask". It is a violation if you start masking changes to an object or performing a calculation that can be described as an operation of the object. For example:

Point p;

// Violation
p.x = p.x + 10;

// Also violation
p.setX(p.getX() + 10);

// Good
p.move(10, 0);.

Coming back to your two objects example

if(a.isX && b.isP){
   a.doY();
   b.doQ();
}

That is not a violation of the "Tell, don't ask" rule. The a.isX and b.isP access to data members is a violation of encapsulation, but that can be easily resolved with accessors is each class.

1
  • 3
    It is a violation if doY() changes state. Then the code is making a decision to change state, based on its state. Wether thru a field, property or other method doesn’t matter.
    – Rik D
    Commented Feb 26 at 18:03
-1

When you do decide it is worth the hassle, there is a refactoring for this that follows tell don't ask. Yes, even with two classes. Yes, without encapsulation destroying accessors. No getters required.

What you need is multiple dispatch. Something which many languages don't directly support. Including Java.

You can simulate multiple dispatch in Java using a little thing called the visitor pattern:

class A {
    private boolean isX;

    public void accept(B b) {
        if (this.isX) {
            b.accept(A::doY);
        }
    }

    public void doY(){
    }
}

Do it this way and you need no accessors. A decides what to do based on it's own state. B decides what to do based on it's own state. And nothing is done until both states have had their say.

Oh sure, it's ugly. It always is when simulating a feature your language doesn't have. But if real encapsulation is important to you, done with no getters, where only A knows isX exists, then this works.

There are slicker solutions that might be viable where A and B don't even know each other exist. See Predicates. See also: Groovy. See also getThis() trick.

3
  • 1
    Just looking at the code snippit from a function naming perspective, this example is either doing something brilliant or missing part of the visitor pattern, and I can't tell which. Using the "standard" terminology from the linked Wiki page, A has an "accept" function, which means it is an Element. "accept" takes a visitor as an argument, which means B must be a visitor. But B also has an accept function, which takes a function as an argument (I'm guessing that's newer Java syntax than I'm familiar with), but a function isn't a visitor. Does B have a method named "accept" (which isn't a..
    – Cort Ammon
    Commented Feb 28 at 13:21
  • .. Element accept function and thus not part of the Visitor pattern), that is doing something really clever here?
    – Cort Ammon
    Commented Feb 28 at 13:22
  • @CortAmmon Java doesn’t have multiple dispatch. Not even double dispatch. It does have single dispatch. Which you can use to simulate it. One way to do that is the visitor pattern. Control goes through single polymorphic dispatching twice. The weird wrinkle is passing the static method reference to achieve the late binding this refactoring needs. That part isn’t classic visitor. It could be done in a number of other ways. Commented Feb 28 at 15:45

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