In this recipe, we will focus on fixing the wrong design of an application that performs logic based on the types of passed objects. The instanceof operator distinguishes the types, and then class casting takes place.
As we did in the previous recipe, let's assume that our system under test is a system that generates a new identity for a given person who can have a name, an age, and siblings, and who sends a JSON message over a web service. Note that the following snippet presents a poorly designed class (all the test examples are written for JUnit; please refer to Chapter 1, Getting Started with Mockito, for the TestNG configuration and Chapter 7, Verifying Behavior with Object Matchers, for the AssertJ configuration and the BDDAssertions static imports):
public class AwefullyCastingNewPersonGenerator {
private final IdentityCreator identityCreator;
public AwefullyCastingNewPersonGenerator(IdentityCreator identityCreator) {
this.identityCreator = identityCreator;
}
public Person generateNewIdentity(Person person) {
String newName = identityCreator.createNewName(person);
int newAge = identityCreator.createNewAge(person);
List<Person> newSiblings = identityCreator.createNewSiblings(person);
Person newPerson = new Person(newName, newAge, newSiblings);
if (identityCreator instanceof PersonDataUpdater) {
((PersonDataUpdater) identityCreator).updatePersonData(newPerson);
}
return newPerson;
}
}The IdentityCreator interface has only three methods: createNewName(...), createNewAge(...), and createNewSiblings(...). There is also another interface called PersonDataUpdater that has the updatePersonData(...) method. Let's assume that there is a class that implements both of these interfaces, as shown in the following code:
public class SingleResponsibilityBreakingNewIdentityCreator implements PersonDataUpdater, IdentityCreator {
public static final String DEFAULT_NEW_NAME = "NewName";
@Override
public String createNewName(Person person) {
System.out.printf("Calling web service and creating new name for person [%s]%n", person.getName());
return DEFAULT_NEW_NAME;
}
@Override
public int createNewAge(Person person) {
System.out.printf("Calling db and creating new age for person [%s]%n", person.getName());
return person.getAge() + 5;
}
@Override
public List<Person> createNewSiblings(Person person) {
System.out.printf("Making heavy IO operations andcreating new siblings for person [%s]%n", person.getName());
return asList(new Person("Bob"), new Person("Andrew"));
}
@Override
public void updatePersonData(Person person) {
String json = buildJsonStringToPerformTheUpdate(person);
System.out.printf("Calling web service to updatenew identity for person [%s] with JSON String [%s]%n", person.getName(), json);
}
private String buildJsonStringToPerformTheUpdate(Person person) {
return "{"name":""+person.getName()+"","age":""+person.getAge()+""}";
}
}In order to test the preceding implementation and then refactor it, we have to perform the following steps:
- Write a test that verifies the behavior of the system under test (in this case, we want to be sure that the person has a new identity at the end of the day).
- Mock out all of the external dependencies (if you know what you are doing, you can move to the next point without performing this intermediary step).
- Refactor the code so that it follows good coding practices, including the SOLID principles.
Let's first write a test for the current implementation that tests the behavior (the person's got a new identity), using the following code:
@RunWith(MockitoJUnitRunner.class)
public class AwfullyCastingNewPersonGeneratorTest {
@Mock(extraInterfaces = PersonDataUpdater.class) IdentityCreator identityCreator;
@InjectMocks AwefullyCastingNewPersonGenerator systemUnderTest;
@Test
public void should_return_person_with_new_identity () {
// given
List<Person> siblings = asList(new Person("John", 10),new Person("Maria", 12));
Person person = new Person("Robert", 25, siblings);
// when
Person newPerson = systemUnderTest.generateNewIdentity(person);
// then
then(newPerson).isNotNull().isNotEqualTo(person);
then(newPerson.getAge()).isNotEqualTo(person.getAge());
then(newPerson.getName()).isNotEqualTo(person.getName());
then(newPerson.getSiblings()).doesNotContainAnyElementsOf(siblings);
}
}Now, since we wrote the missing test, we can refactor the code with a greater sense of confidence. We have to clearly split the system under test in such a way that it does not use class casts but proper injected dependencies. Take a look at the following code:
public class RefactoredNewPersonGenerator {
private final IdentityCreator identityCreator;
private final PersonDataUpdater personDataUpdater;
public RefactoredNewPersonGenerator(IdentityCreator identityCreator, PersonDataUpdater personDataUpdater) {
this.identityCreator = identityCreator;
this.personDataUpdater = personDataUpdater;
}
public Person generateNewIdentity(Person person) {
String newName = identityCreator.createNewName(person);
int newAge = identityCreator.createNewAge(person);
List<Person> newSiblings = identityCreator.createNewSiblings(person);
Person newPerson = new Person(newName, newAge, newSiblings);
personDataUpdater.updatePersonData(newPerson);
return newPerson;
}
}Check the current test pass when it uses class casts, when it is possible to clean up the test. Now, the test looks simpler. Take a look at the following code:
@RunWith(MockitoJUnitRunner.class)
public class RefactoredPersonGeneratorTest {
@Mock IdentityCreator identityCreator;
@Mock PersonDataUpdater personDataUpdater;
@InjectMocks RefactoredNewPersonGenerator systemUnderTest;
@Test
public void should_return_person_with_new_identity () {
// given
List<Person> siblings = asList(new Person("John", 10), new Person("Maria", 12));
Person person = new Person("Robert", 25, siblings);
// when
Person newPerson = systemUnderTest.generateNewIdentity(person);
// then
then(newPerson).isNotNull().isNotEqualTo(person);
then(newPerson.getAge()).isNotEqualTo(person.getAge());
then(newPerson.getName()).isNotEqualTo(person.getName());
then(newPerson.getSiblings()).doesNotContainAnyElementsOf(siblings);
}
}- Refer to Refactoring: Improving the Design of Existing Code, Martin Fowler (with Kent Beck, John Brant, William Opdyke, and Don Roberts), available at http://martinfowler.com/books/refactoring.html
- Refer to Working Effectively with Legacy Code, Martin Feathers, available at http://www.amazon.com/Working-Effectively-Legacy-Michael-Feathers/dp/0131177052
- Refer to Martin Fowler's catalog of refactoring methods at http://refactoring.com/catalog/