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(); } }

5 Answers5

29

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.

psmears
  • 196
  • 4
Doc Brown
  • 218,378
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.

Christophe
  • 81,699
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.

Cort Ammon
  • 11,917
  • 3
  • 26
  • 35
2
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

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.

candied_orange
  • 119,268