Is 'facade method' (calling public methods of own class) considered bad design?

Question

Is it considered bad design to have a 'facade method' (I can't think of a better term). Example:

public class MyService() {
    public void doThis() {
        // do something
    }

    public int doThat() {
       // do something
    }

    public boolean isThisTrue(String str) {
       // do something
    }

    // determine something by calling methods within the same class
    public boolean shouldSomethingBeDone() {
        int result = 0;            

        if (isThisTrue("foobar")) {
           doThis();
           result = doThat();
        }

        return result > 0; 
    }
}

I'm trying to avoid having too much logic in one of my Controller classes that is calling a service. The only way I could see getting around this was by created a method such as the one above.

The dilemma arose when I tried to create a unit test and mock the calls within the shouldSomethingBeDone() method. Since all of the calls are to methods within the same class it's difficult to mock this.

Thanks.

Answer 1

Have you seen the Tell Don't Ask principle?

Methods like isThisTrue ask questions and push the logic for answering them all over the place. Instead, prefer to just tell the object what to do and let it worry about it.

This suggests that instead of shouldSomethingBeDone you should rename the method to tell the object what to do instead (difficult without real names, but let's say doSomething is a better name).

Answer 2

In the highly abstracted version that you have given, it is hard to say whether this is an example of good design or bad design. However, I don't think there is general principle that says it is bad.

The fact that something might be difficult to test (exhaustively) doesn't make it bad design ... or vice versa.

We could critique the names of those methods, but I don't think that addresses your question, with respect to either design or testability.

Answer 3

Structuring your code using methods is generally good design. Of course you shouldn't overdo like creating lots of one-line methods.

If you have a method, which shouldn't be called by another class, just don't use public and use private.

Another advantage: Using Inheritance might be easier, because you might not need to override whole shouldSomethingBeDone(), but just override smaller methods.

Yeah, mocking is difficult. I would test shouldSomethingBeDone() Method as a whole. Or just use Google to find out how to mock a method in the same class you are testing.