Androidsx | android ninjas

Don’t look for things. Ask for them!

November 26, 2009 at 12:04 pm | blog, software quality | 3 comments

Before our previous article, our code to model a mechanic used to look like this:

class Mechanic {
    private final Engine engine = ServiceLocator.getCar().getEngine();
    public void fixEngine() { /* ... */ }
}

Luckily, we applied some dependency injection, resulting in nicer code: testable, and with an explicit dependency on the ServiceLocator:

class Mechanic {
    private final Engine engine;
    public Mechanic(final ServiceLocator serviceLocator) {
        engine = serviceLocator.getCar().getEngine();
    }
    public void fixEngine() { /* ... */ }
}

However, does our mechanic care about the ServiceLocator at all? Not really, it doesn’t even store a reference to it! The mechanic just wants an engine.

There are some problems with this kind of code:

• The ServiceLocator probably has references to a lot of other classes in the system. By the transitive property, our Mechanic class is too coupled with the rest of the system: if you want to reuse it in a different project, you’d need to reference not only Mechanic and Engine, but also the ServiceLocator with all its dependencies.
• The API is not clear. An API client knows that it’d need to provide the ServiceLocator (already an advantage respect to the Singleton-based case). What it doesn’t know is what the mechanic really needs (an engine). This fact is hidden in the source code.
• The tests contain a lot of setup junk that masks its real purpose. For every test of the mechanic, you’d need to mock the ServiceLocator, and then make sure the appropriate reference can be retrieved from it (the Engine, in this case). When we test the class Garage, that needs a Mechanic, we’ll have to replicate this tedious setup there too.

This is how the test looks like now:

@Test
public void testCantFixBrokenDownEngine() {
    final Engine engine = EngineFactory.buildBrokenEngine();
    final Car mockCark = Mockito.mock(Car.class);
    Mockito.when(mockCar.getEngine()).thenReturn(engine);
    final ServiceLocator mockServiceLocator = Mockito.mock(ServiceLocator.class);
    Mockito.when(mockServiceLocator.getCar()).thenReturn(car);
    final Mechanic mechanic = new Mechanic(mockServiceLocator);
    mechanic.fixEngine();
    assertFalse(engine.works());
}

Why not just ask for what you need?

class Mechanic {
    private final Engine engine;
    public Mechanic(final Engine engine) {
        this.engine = engine;
    }
    public void fixEngine() { /* ... */ }
}

@Test
public void testCantFixBrokenDownEngine() {
  Engine engine = EngineFactory.buildBrokenEngine();
  Mechanic mechanic = new Mechanic(engine);
  mechanic.fixEngine();
  assertFalse(engine.works());
}

Everyone wins:

API writers:

• Unaffected by ServiceLocator changes
• In-code documentation is easier to write (what was your comment on the Mechanic‘s constructor for the ServiceLocator)?

Test writers:

• Tests are easy to read
• Very little setup code
• Unaffected by ServiceLocator changes

API users:

• The API is clear: the mechanic needs an engine

Related posts

• Can’t test that Singleton? Try Dependency Injection!
• New features in Java 7
• Stop the spread of nulls
• How to write untestable code

<< Back to Blog Discuss this post