2

I've just finished reading The Art of Unit Testing, by Roy Osherove. It was an interesting book, but I'm having trouble with something he mentions near the end (section 11.2.2):

Identifying "roles" in the application and abstracting them under interfaces is an important part of the design process. An abstract class shouldn’t call concrete classes, and concrete classes shouldn’t call concrete classes either, unless they’re data objects (objects holding data, with no behavior). This allows you to have multiple seams in the application where you could intervene and provide your own implementation.

Specifically, I don't understand what he means when he says that concrete classes shouldn't call concrete classes. Consider the following examples in C#:

double roundedAge = Math.Floor(exactAge);

Is this not considered a call to a class because Math.Floor is a static method? Or is Osherove saying that the line of code above is bad design? Another example:

using (StreamReader reader = new StreamReader(path))
{
    // do things
}

Is this bad design due to the use of a StreamReader? Or is this instance of StreamReader simply an object that holds data, as described in the quote above?

Rather than starting a discussion on whether or not it's bad practice to have classes call other classes, my goal here is to try and better understand what Osherove means in the passage I've quoted. Does he really mean that the code in the two examples above should be avoided? Thanks in advance for any replies.

EDIT: Below is an illustration from the book which is possibly elucidating.

Nice

closed as unclear what you're asking by Robert Harvey, whatsisname, user40980, jwenting, amon May 4 '14 at 10:34

Please clarify your specific problem or add additional details to highlight exactly what you need. As it's currently written, it’s hard to tell exactly what you're asking. See the How to Ask page for help clarifying this question. If this question can be reworded to fit the rules in the help center, please edit the question.

  • 2
    You haven't provided enough context from the book to make your question answerable. I have pretty good domain knowledge in this area, but I don't understand what this paragraph from the book is saying in isolation. Did Roy provide any code examples that illustrate this problem? – Robert Harvey May 1 '14 at 23:41
  • 1
    @RobertHarvey The quote comes from a section called "Use interface-based designs", and unfortunately, the quote is the entirety of the section (it's a very short section). He does refer to a number of examples from several chapters earlier in the book, but none of them really address this quote specifically. For reference, the quote in question is from Section 11.2.2 of the book. – Brian Snow May 2 '14 at 0:09
  • 2
    They don't address it exactly, but they do make it clear. I would suggest to add a few of those diagrams like Figure 3.4 or Figure 4.3 (they are the clearest examples of adding an intermediate interface instead of a concrete class). You can also get in contact with him on his blog: osherove.com/request-more-info – Jeroen Vannevel May 2 '14 at 0:13
  • 1
    I have my doubts whether that should really be interpreted as "always, never different" or if it's simply an overstatement on his part. It feels like an exagerated "program towards an interface" idea. – Jeroen Vannevel May 2 '14 at 0:23
  • 2
    OK, I got my copy of TAOUT, and noticed that the title of the section you quoted is "Use Interface-based Designs." That's it, in a nutshell. Using interfaces allows you to swap out the implementation for a mock object; that's what he means by "seams." – Robert Harvey May 2 '14 at 4:21
6

disclaimer: I didn't check that any of the code in this answer actually compiles - it's only there to demonstrate a point.

I believe that the worst anti-pattern is Cargo Cult Programming - blindly following design patterns. I claim that it's much worse than any other anti-pattern, because while the other anti-pattern at least indicate some (wrong) thought process from the applier, Cargo Cult Programming shuts the mind down so it won't interrupt in the act of applying needless design patterns.

In our case, it seems like the Dependency Inversion pattern is being cargo-culted. Using DI for every class(save for data-only classes) is impossible, as at some point concrete classes are needed to be constructed. At the very least, some static methods and all factory classes should be exempted from this rule.

This exemption will make the task possible - but still needlessly hard. The key behind using Dependency Injection properly is understanding that you need to decouple modules(or components), not classes. Classes of the same module should be allowed to use other concrete classes of the same module. Of course - deciding where the boundaries between components go - which classes belong to each component - requires thought, something that is forbidden in cargo cults.

Another important thing is that some things are too small for can-be-used-by-interface-only, and some things are too high for the only-use-interfaces.

Your Math.Floor example is and example of "too small for can-be-used-by-interface-only". Inversing the dependency here will require something like:

interface IMath{
    public double Floor(double d);
}

public void Foo(IMath math){
    // ...
    double roundedAge=math.Floor(exactAge);
    // ...
}

This allows us to do things like:

  • Call Foo with some other implementation of IMath that implements Floor differently
  • Mock IMath in unit tests - call Foo with an IMath object that doesn't really do the flooring, only pretends that it does.

You need to be very deep in the cargo cult to ignore the fact that these two benefits are completely useless here. It's hard to imagine a method that does Floor better than System.Math.Floor, and any attempt to mock it - unless it only works for specific cases - will be more complicated and error-prune than just doing the damn flooring.

Compare with the costs:

  • Having to send a IMath implementation to Foo.
  • Foo's implementation(using a concrete IMath object) is exposed by it's interface(requires an IMath object as argument).
  • Every method that uses Foo will also have these costs, since it can't use a concrete IMath directly.

and it's easy to see that it just ain't worth it in this case. An example of "too high for the only-use-interfaces" is your Main method, which needs to actually create concrete objects that can't be injected to it.

Your StreamReader example is actually a good example for something that can possibly benefit from Dependency Inversion:

  • It's possible that you'll want to provide a different implementation of TextReader - for example one that reads and decrypts an encrypted stream.
  • It's possible that you'll want to mock TextReader in a unit test, so you wouldn't need an actual file to exist in a specific path when you run your unit test and can instead supply the text that would be read directly in the unit test.

Still - at some point a concrete object needs to be created. Here we should find a place that's "too high for the only-use-interfaces", where we can create the TextReader or a factory object that can create a TextReader and inject it down until it's used:

interface ITextReaderFactory{
    TextReader CreateTextReader(string path);
}


public void Bar(ITextReaderFactory textReaderFactory){
    // ...
    using(TextReader reader=textReaderFactory.CreateTextReader(path)){
        // do things
    }
    // ...
}

class StreamReaderFactory : ITextReaderFactory{
    public TextReader CreateTextReader(string path){
        return new StreamReader(path);
    }
}

public static void Main(string[] args){
    Bar(new StreamReaderFactory());
}
  • This was very helpful, thanks for taking the time to write it! – Brian Snow May 2 '14 at 1:05
  • I still don't understand what any of this has to do with abstract classes calling concrete classes, or concrete classes calling other concrete classes unless they are data classes. My primary beef is with the terminology "calling a concrete class." There is no such thing. You can call a method on a class instance. You can call a static method on a static class. Byt you cannot call a class, no matter what anyone says. But you could call it Ray or Jay. – Robert Harvey May 2 '14 at 2:40
  • @RobertHarvey That was part of what I was confused about. I don't know enough about programming to know whether or not the terminology he uses in the quote is common or not, so that's part of the reason why I asked the question. – Brian Snow May 2 '14 at 4:41
  • 1
    @RobertHarvey From the context I think the book talks about constructing concrete classes and using a concrete class as a type for variables, arguments, fields and/or return values. – Idan Arye May 2 '14 at 10:24
0

The first example does not apply since it is essentially an arithmetic expression. You seldom write unit tests for arithmetic operations; when you do, your approach would be quite different.

In the second example, your code is using the concrete class. This means that the code inside the curly braces is harder to test since you are binding the StreamReader at compile time. What the author is describing is to defer the binding of reader as late as possible. When you do that, it becomes possible to test the use of reader with less pain.

That being said, many of the code mocking tools do a sufficiently good job to allow substitution of the mocked item for the real thing. This is the intent of the prescription: keep as many useful "seams" as you can in your code, because you do all of your testing at the seams.

  • What did Roy mean in his book when he said "Abstract classes should not call concrete classes, and concrete classes should not call other concrete classes unless they are data objects?" – Robert Harvey May 2 '14 at 0:05
  • I don't know about the first part. The second part refers to the business with seams. If a concrete class invokes a method of another concrete class, then it may make the first class less testable. As I suggest, I don't necessarily agree with this, since the use of factories and mocks can often work around those issues. – BobDalgleish May 2 '14 at 2:27

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