Mocking concrete class - Not recommended

Question

I've just read an excerpt of "Growing Object-Oriented Software" book which explains some reasons why mocking concrete class is not recommended.

Here some sample code of a unit-test for the MusicCentre class:

public class MusicCentreTest {
  @Test public void startsCdPlayerAtTimeRequested() {
    final MutableTime scheduledTime = new MutableTime();
    CdPlayer player = new CdPlayer() { 
      @Override 
      public void scheduleToStartAt(Time startTime) {
        scheduledTime.set(startTime);
      }
    }

    MusicCentre centre = new MusicCentre(player);
    centre.startMediaAt(LATER);

    assertEquals(LATER, scheduledTime.get());
  }
}

And his first explanation:

The problem with this approach is that it leaves the relationship between the objects implicit. I hope we've made clear by now that the intention of Test-Driven Development with Mock Objects is to discover relationships between objects. If I subclass, there's nothing in the domain code to make such a relationship visible, just methods on an object. This makes it harder to see if the service that supports this relationship might be relevant elsewhere and I'll have to do the analysis again next time I work with the class.

I can't figure out exactly what he means when he says:

This makes it harder to see if the service that supports this relationship might be relevant elsewhere and I'll have to do the analysis again next time I work with the class.

I understand that the service corresponds to MusicCentre's method called startMediaAt.

What does he mean by "elsewhere"?

The complete excerpt is here: http://www.mockobjects.com/2007/04/test-smell-mocking-concrete-classes.html

Answer 1

The author of that post is promoting the use of Interfaces over the use of member classes.

It turns out that my MusicCentre object only uses the starting and stopping methods on the CdPlayer, the rest are used by some other part of the system. I'm over-specifying my MediaCentre by requiring it to talk to a CdPlayer, what it actually needs is a ScheduledDevice.

The relationship that he's worried about re-discovering later is the fact that the MediaCentre class doesn't need all of the CdPlayer object. His claim is that by using an Interface (presumably limited to just start | stop) that it's easier to understand what the interaction really is.

"elsewhere" simply means that other objects may have similarly limited relationships, and the full member object is not required - a subset of the functionality wrapped through an Interface should be sufficient.

The claim starts to make more sense when you explode out all the potential functionality:

• start
• stop
• pause
• record
• random play order
• sample tracks, beginning of song
• sample tracks, random sample of song
• provide media information
• ...

Now his claim of "I just need start & stop" makes more sense. Using the concrete member object instead of an Interface makes it less clear to future developers as to what is really required. Running unit tests from MediaCentre on all the other functions within CdPlayer is a waste of testing effort since they belong to the "don't care" state. If the Record function wasn't working in this case, we truly don't care since it isn't required. But a future maintainer wouldn't necessarily know that based upon the code, as written.

Ultimately, the author's premise is to use only what is needed and make it clear to future maintainers what was required before. The goal is to minimize rework / reanalyzing the module of code during subsequent maintenance.

Answer 2

This makes it harder to see if the service that supports this relationship might be relevant elsewhere and I'll have to do the analysis again next time I work with the class.

After thinking a lot about it, I get a possible interpretation of this quote:

The quoted "service" corresponds to the "fact of scheduling". This could be expressed by a well-named and "focused-on-one-role" interface named "ScheduledDevice" or expressed implicitly by a concrete method implementation not depending upon any interfaces.

In the sample above, scheduling is expressed by the whole full-featured object named CDPlayer. Thus, it still leads to implicit relationship between MusicCentre and "fact of scheduling".

So if we start injecting concrete classes and mocking them into high-level objects; when we want to test these ones, we have to analyse each injected "concrete" objects to see whether it presents a specific relationship that we HAVE TO MOCK because they are HIDDEN (implicit). On the contrary, coding ALWAYS over interface allows developer to figure out directly what kind of relationship is about to be served by the high-level object and therefore detect features that have to be mocked in order to isolate the unit-test.

Answer 3

The service I meant here was CDPlayer.scheduleToStartAt(). That's what the MediaCentre calls--the collaborator it needs to function. The MediaCentre is the object under test.

The idea is that if I make explicit just what the MediaCentre depends on, not an implementation class, I can give that dependency role a name and talk about it. All the MediaCentre needs to know is that it talks to ScheduledDevices. As rest of the system changes, I won't need to alter the MediaCentre unless its features change.

Does that help?