"Fear is the path to the dark side. Fear leads to anger. Anger leads to hate. Hate leads to suffering." | ||
| --Yoda | ||
TDD might not adjust straightaway to legacy code. You may have to fiddle a bit with the steps to make it work. Understand that your TDD might change in this case. That, somehow, you are no longer performing the TDD you were used. This chapter will introduce you to the world of legacy code, taking as much as we can from TDD.
We'll start a fresh, with a legacy application that is currently in production. We'll alter it in small ways without introducing defects or regressions and we'll even have time to have an early lunch!
The following topics are covered in this chapter:
- Legacy code
- Dealing with legacy code
- REST communication
- Dependency injection
- Tests at different levels: end to end, integration, and unit
Let's start with the definition of legacy code. While there are many authors with different definitions such as lack of trust in your application or your tests, code that is no longer supported, and so on, we like the one created by Michael Feathers the most:
Legacy code is code without tests. The reason for this definition is that it is objective: either there are or there aren't tests. | ||
| --Michael Feathers | ||
How to detect legacy code? Although legacy code usually frequents bad code, Michael Feathers exposes some smells in his book Working Effectively with Legacy Code by Dorling Kindersley (India) Pvt. Ltd. (1993).
Tip
Code Smell
Smells are certain structures in the code that indicate violation of fundamental design principles and negatively impact design quality.
Code smells are usually not bugs—they are not technically incorrect and do not currently prevent the program from functioning. Instead, they indicate weaknesses in design that may be slowing down development or increasing the risk of bugs or failures in the future.
One of the common smells for legacy code is: I can't test this code. It is accessing outside resources, introducing other side-effects, using a new operator, and so on. In general, good design is easy to test. Let's see some legacy code.
Software concepts are often easiest to explain through code and this one is no exception. We have seen and worked with the Tic-Tac-Toe application (see Chapter 3, Red-Green-Refactor – from Failure through Success until Perfection). The following code performs position validation:
public class TicTacToe {
public void validatePosition(int x, int y) {
if (x < 1 || x > 3) {
throw new RuntimeException("X is outside "
+ "board");
}
if (y < 1 || y > 3) {
throw new RuntimeException("Y is outside "
+ "board");
}
}
}The specification that corresponds with this code is as follows:
public class TicTacToeSpec {
@Rule
public ExpectedException exception =
ExpectedException.none();
private TicTacToe ticTacToe;
@Before
public final void before() {
ticTacToe = new TicTacToe();
}
@Test
public void whenXOutsideBoardThenRuntimeException()
{
exception.expect(RuntimeException.class);
ticTacToe.validatePosition(5, 2);
}
@Test
public void whenYOutsideBoardThenRuntimeException()
{
exception.expect(RuntimeException.class);
ticTacToe.validatePosition(2, 5);
}
}The JaCoCo report indicates that everything is covered (except the last line, the method's closing bracket):
As we believe we have good coverage, we can perform an automatic and safe refactor (fragment):
public class TicTacToe {
public void validatePosition(int x, int y) {
if (isOutsideTheBoard(x)) {
throw new RuntimeException("X is outside "
+ "board");
}
if (isOutsideTheBoard(y)) {
throw new RuntimeException("Y is outside "
+ "board");
}
}
private boolean isOutsideTheBoard
(final int position) {
return position < 1 || position > 3;
}
}This code should be ready, as the tests are successful and it has a very good test coverage.
Maybe you have already realized as much, but there is a catch. The message in the RuntimeException block is not checked for correctness; even the code coverage shows it as "covered all the branches in that line".
Note
What is coverage all about?
Coverage is a measure used to describe the degree to which the source code of a program is tested by a particular test suite. Source: http://en.wikipedia.org/wiki/Code_coverage.
Let's imagine a single end-to-end test that covers an easy part of the code. This test will get you a high coverage percentage, but not much security, as there are many other parts that are still not covered.
We have already introduced legacy code in our codebase: the exception messages. There might be nothing wrong with this as long as this is not an expected behavior: no one should depend on exception messages, not programmers to debug their programs, or logs, or even users. Those parts of the program that are not covered by tests are likely to suffer regressions in the near future. This might be fine if you accept the risk. Maybe the exception type and the line number are enough.
We have decided to remove the exception message, as it is not tested:
public class TicTacToe {
public void validatePosition(int x, int y) {
if (isOutsideTheBoard(x)) {
throw new RuntimeException("");
}
if (isOutsideTheBoard(y)) {
throw new RuntimeException("");
}
}
private boolean isOutsideTheBoard
(final int position) {
return position < 1 || position > 3;
}
}You might be familiar with some of the following common signs of legacy applications:
- A patch on top of a patch, just like a living Frankenstein application
- Known bugs
- Changes are expensive
- Fragile
- Difficult to understand
- Old, outdated, static or, often, non-existent documentation
- Shotgun surgery
- Broken windows
Regarding the team that maintains it, these are some of the effects it produces on them:
- Resignation: The people in charge of the software see a huge task in front of them
- No one cares anymore: If you already have broken windows in your system, it is easier to introduce new ones
As legacy code is usually more difficult than other kinds of software, you would want your best people to work on it. However, we are often in a hurry imposed by deadlines, with the idea of programming required functionalities as fast as possible and ignoring the quality of the solution.
Therefore, to avoid wasting our talented developers in such a bad way, we expect a non-legacy application to fulfill just the opposite. It should be:
- Easy to change
- Generalizable, configurable, and expansible
- Easy to deploy
- Robust
- No known defects or limitations
- Easy to teach to others/to learn from others
- Extensive suite of tests
- Self-validating
- Able to use keyhole surgery
As we have outlined some of the properties of legacy and non-legacy code, it should be easy to replace some qualities with others. Right? Stop shotgun surgery and use keyhole surgery, a few more details and you are done. Isn't that right?
It is not as easy as it sounds. Luckily there are some tricks and rules that, when applied, improve our code and the application comes closer to a non-legacy one.
This is one of the smells often detected in a legacy codebase. As there is no need to test the classes in isolation, the collaborators are instantiated where they are needed, putting the responsibility of creating collaborators and using them in the same class.
An example, using the new operator:
public class BirthdayGreetingService {
private final MessageSender messageSender;
public BirthdayGreetingService() {
messageSender = new EmailMessageSender();
}
public void greet(final Employee employee) {
messageSender.send(employee.getAddress(), "Greetings on your birthday");
}
}In the current state, the service BirthdayGreeting is not unit-testable. It has the dependency to EmailMessageSender hardcoded in the constructor. It is not possible to replace this dependency without modifying the code-base (except for injecting objects using reflection or replacing objects on the new operator).
Modifying the codebase is always a source of possible regressions, so it should be done with caution. Refactoring requires tests, except when it is not possible.
When you have to make a change in a legacy code base, here is an algorithm you can use:
In order to apply this algorithm, we usually start with a suite of tests and always keep it green while refactoring. This is different from the normal cycle of TDD because refactoring should not introduce any new feature (that is, it should not write any new specifications).
In order to better explain the algorithm, imagine that we received the following change request:
To greet my employees in a more informal way, I want to send them a tweet instead of an email.
The system is only able to send emails right now, so a change is necessary. Where? A quick investigation shows that the strategy for sending the greeting is decided in the constructor for the BirthdayGreetingService class following the strategy pattern https://en.wikipedia.org/?title=Strategy_pattern (fragment):
public class BirthdayGreetingService {
public BirthdayGreetingService() {
messageSender = new EmailMessageSender();
}
[...]
}As the BirthdayGreetingService class does not have any collaborator injected that could be used to attach additional responsibilities to the object, the only option is to go outside this service class to test it. An option would be to change the EmailMessageSender class for a mock or fake implementation, but this would risk the implementation in that class.
Another option is to create an end-to-end test for this functionality:
public class EndToEndTest {
@Test
public void email_an_employee() {
final StringBuilder systemOutput =
injectSystemOutput();
final Employee john = new Employee(
new Email("[email protected]"));
new BirthdayGreetingService().greet(john);
assertThat(systemOutput.toString(),
equalTo("Sent email to "
+ "'[email protected]' with "
+ "the body 'Greetings on your "
+ "birthday'
"));
}
// This code has been used with permission from
//GMaur's LegacyUtils:
// https://github.com/GMaur/legacyutils
private StringBuilder injectSystemOutput() {
final StringBuilder stringBuilder =
new StringBuilder();
final PrintStream outputPrintStream =
new PrintStream(
new OutputStream() {
@Override
public void write(final int b)
throws IOException {
stringBuilder.append((char) b);
}
});
System.setOut(outputPrintStream);
return stringBuilder;
}
}This code has been used with permission from https://github.com/GMaur/legacyutils. This library helps you perform the technique of capturing the System out (System.out).
The name of the file does not end in Specification (or Spec), such as TicTacToeSpec, because this is not a specification. It is a test, to ensure the functionality remains constant. The file has been named EndToEndTest because we try to cover as much functionality as possible.
After having created a test that guarantees the expected behavior does not change, we will break the hardcoded dependency between BirthdayGreetingService and EmailMessageSender. For this, we will use a technique called Extract and Override Call, which is first explained in Michaels Feathers' book:
public class BirthdayGreetingService {
public BirthdayGreetingService() {
messageSender = getMessageSender();
}
private MessageSender getMessageSender() {
return new EmailMessageSender();
}
[...]Rerun the tests and the single test we have is green. We need to make this method protected or more open to be able to override it:
public class BirthdayGreetingService {
protected MessageSender getMessageSender() {
return new EmailMessageSender();
}
[...]We create a fake in the test folder. Introducing fakes in code is a pattern that consists of creating an object that could replace an existing one with the particularity that we can control its behavior. This way, we can inject some customized fakes to achieve what we need. More information is available at http://xunitpatterns.com/.
In this particular case, we should create a fake service that extends the original service. The next step is to override complicated methods in order to bypass irrelevant parts of code for testing purposes:
public class FakeBirthdayGreetingService extends BirthdayGreetingService {
@Override
protected MessageSender getMessageSender() {
return new EmailMessageSender();
}
}Now we can use the fake, instead of the BirthdayGreetingService class:
public class EndToEndTest {
@Test
public void email_an_employee() {
final StringBuilder systemOutput =
injectSystemOutput();
final Employee john = new Employee(
new Email("[email protected]"));
new FakeBirthdayGreetingService().greet(john);
assertThat(systemOutput.toString(),
equalTo("Sent email to "
+ "'[email protected]' with "
+ "the body 'Greetings on "
+ "your birthday'
"));
}We can now apply another dependency-breaking technique "Parameterize Constructor", explained in Feathers' paper http://www.objectmentor.com/resources/articles/WorkingEffectivelyWithLegacyCode.pdf. The production code may look like this:
public class BirthdayGreetingService {
public BirthdayGreetingService(final MessageSender messageSender) {
this.messageSender = messageSender;
}
[...]
}Test code that corresponds to this implementation can be as follows:
public class EndToEndTest {
@Test
public void email_an_employee() {
final StringBuilder systemOutput =
injectSystemOutput();
final Employee john = new Employee(
new Email("[email protected]"));
new BirthdayGreetingService(new EmailMessageSender()).greet(john);
assertThat(systemOutput.toString(),
equalTo("Sent email to "
+ "'[email protected]' with "
+ "the body 'Greetings on "
+ "your birthday'
"));
}
[...]While keeping the old end-to-end test, create an interaction to verify the integration of BirthdayGreetingService and MessageSender:
@Test
public void the_service_should_ask_the_messageSender() {
final Email address =
new Email("[email protected]");
final Employee john = new Employee(address);
final MessageSender messageSender =
mock(MessageSender.class);
new BirthdayGreetingService(messageSender)
.greet(john);
verify(messageSender).send(address, "Greetings on your birthday");
}At this point, a new TweetMessageSender can be written, thus completing the last step of the algorithm.