Clean Code – Dependency Injection Use in Robert Martin’s Clean Code

clean codedependency-injectioninversion-of-control

In Robert Martin's Clean Code, I encountered the following method on p. 195:

private void parseSchemaElement(String element) throws ArgsException {
    char elementId = element.charAt(0);
    String elementTail = element.substring(1);
    validateSchemaElementId(elementId);
    if (elementTail.length() == 0) marshalers.put(elementId, new BooleanArgumentMarshaler());
    else if (elementTail.equals("*")) marshalers.put(elementId, new StringArgumentMarshaler());
    else if (elementTail.equals("#")) marshalers.put(elementId, new IntegerArgumentMarshaler());
    else if (elementTail.equals("##")) marshalers.put(elementId, new DoubleArgumentMarshaler());
    else if (elementTail.equals("[*]")) marshalers.put(elementId, new StringArrayArgumentMarshaler());
    else throw new ArgsException(INVALID_ARGUMENT_FORMAT, elementId, elementTail);
}

Just a few pages earlier (p. 157), the book advocates use of dependency injection (DI) in order to structure our program better and facilitate testing in isolation.

Would the above example be the case where dependency injection should be used in order to test the given class in isolation, i.e. without also testing all the *ArgumentMarshaller classes?

We could have a factory object, which would create an appropriate *ArgumentMarshaller where needed – this would be consistent with the DI recommendations and also facilitate testing with mocks/doubles.

..or perhaps did I misunderstand something and the dependency injection applied here is not a good idea?

Best Answer

Simply put, it depends.

Even though I am all for dependency injection in classes containing business logic, for this specific case and without knowing the context, even though the Marshalers are instantiated directly, it seems the only work that is done with them is adding them do a dictionary based on some rule, but not actually performing any operations on them. And for that I would be OK with instantiating the classes directly in place.

If you, however, wanted to decouple the creation of Marshalers in question, the factory approach is the correct one.

Dependency injection is generally recommended for classes which contain methods performing operations with side effects, such as writing to a database. In that case you want to know what the dependencies are so you are not surprised when a method work suddenly wipes your entire database even though, based on public API of the class, it shouldn't have access to it.

The only real improvement I'd have for this code would be to rename the method to parseSchemaElementAndAddItToMarshalerCollection.

Related Topic