Working effectively with legacy code notes

06-I-don't-have-time.md

I don't have much time and I have to change it (p58)

This chapter talks about ways to work around legacy code when you don't have time to refactor first. This happens a lot because it's hard to predict how long adding a feature to legacy code will take, and it's tempting to hack first and leave refactoring till the end (i.e. never).

Before using these techniques, see if you can get the legacy code into a test harness - it may be easier than you think.

If not, these techniques will reduce the risk and/or avoid making the legacy code incrementally worse over time.

The downside compared to refactoring the legacy code is that the legacy code won't go away, and the codebase will accumulate non-legacy bits that duplicate concepts from the legacy code. This is not so bad though, as it encourages further refactoring later as you find yourself reading more tested code.

Changes tend to cluster so if you are working in a particular area today chances are you will need to look at it again soon.

Sprout method

Changes are invasive if they mingle different operations and/or introduce new temporary variables into existing methods

To minimise the invasiveness, first study the existing code and then sprout methods:

1. Figure out where in the source method the change could be made as a sequence of statements
2. Make any local variables it depends on parameters of a new method
3. Determine return value the source method will need
4. TDD the new method
5. Uncomment the call

Disadvantages:

• may introduce temporal coupling i.e. things that are together because they need to be done at the same time, but the relationship between them isn't strong

Sprout class

This is similar to sprout method.

May be motivated by introducing a new responsibility that shouldn't be part of the original, or you can't get the original under test.

Sprouted methods may become part of the original concept (if you get it under test) or might not. You may extract common code as the design evolves

Advantages:

• can get on with the change you want to make with more confidence

Disadvantages:

• increases conceptual complexity
• things that logically should belong in one class are split up

Wrap method

1. Create method with the same name as the old method
2. Delegate to the old code
3. Add a new method

Or, just add a new wrapper method if nothing calls it yet.

Advantages:

• Decouples the two behaviours

Disadvantages:

• You have to make up a new name, which might make less sense than the old one

Wrap class

Use when the behaviour is entirely independent of the class you have to change, or too low level

Or if you have reached the point where you want to make no more changes to a legacy class at all (in this case you should have a roadmap for later changes)

With Extract Implementor (Decorator Pattern)

This pattern allows you to compose objects at runtime

1. Extract an interface from the original implemntation
2. Have the new implementation implement that interface
3. The new implementation can accept another implementor as a parameter
4. New implementation delegates to old implementation

Advantages:

• you can test the new subclass

Disadvantages:

• hard to navigate/debug if there are lots of layers

Without decorators

This is useful if there is only one place the old code is being called

1. Create a new class that wraps the old class
2. Use TDD to create methods on the new class (delegating to the old class as apprioriate)
3. Update the call site to use the new class

07-it-takes-forever.md

It takes forever to make a change (p77)

This chapter is about breaking dependencies to get code into test harness.

It focuses on establishing a fast compile-link-test feedback cycle in compiled languages.

I think this is less of an issue in ruby but is still relevant. We should be able to easily run individual unit tests without loading the whole rails app all the time.

Some characteristics of legacy code

• long time to figure out what you need to do
• making the change is also difficult
• it feels like you haven't learnt much beyond a narrow understanding

Shortening feedback loops

"In most mainstream languages, you can always break dependencies in a way that lets you recompile and run tests against whatever code you're working on in less than 10 seconds."

(this seems harder when we are relying heavily on feature specs in rails to get feedback)

"You should be able to compile every class or module in your system seperately from the others and it its own test harness."

"If we have to perform a short task (5-10 seconds long) and we can only take a step once every minute, we usually do it and then pause. [After planning the next step] our minds wander until we can do the next step. If we compress the time between steps from a minute to a few seconds, the quality of the mental work becomes different. [...] Our work becomes more like driving than waiting at a bus stop."

Compilation in statically typed languages

Once you've got some code into a test harness, changes to those classes will also affect the compilation of anything that depends on those classes. (Is this generally true with modern IDEs/build tools?)

Extracting interfaces can break this build time dependency. Follow the dependency inversion principle

Designs end up with a lot of interfaces and $InterfaceImpl classes (i.e. only a single implementation of the interface).

Can think of this like adding a "compilation firewall" to the code.

In languages like java it's useful to think about dependencies at the package level as well. By breaking dependencies you can trade off conceptual overhead for faster compilation times.

Splitting loosely coupled classes/modules

Can structure builds by moving clusters into new packages or libraries.

Disadvantages:

• Adds build complexity
• Overall build time increases

Advantages:

• Average build time decreases

Reflections on monolition project

We have split off contexts which shouldn't depend on the rest of the monolith. But it's still part of the same project at build time, and we still have dependencies at the model level. If we broke that dependency these could be packaged up seperately and/or tested seperately. In this case we could be a lot more confident about one day slicing these into seperate runtime services as well.

Further reading

Author recommends Agile Software Development: Principles, Patterns and Practices

08-how-do-i-add-a-feature.md

How do I add a feature? (p87)

TDD.

The difference for legacy code is there is an initial step of getting the code you want to change under test.

Programming by difference

1. Use inheritence to add new behaviour in a new subclass
2. Then decide how this should be integrated in the code
3. Then refactor to a simpler design

This can violate the Liskov substitution principle because you are overriding concrete methods. The meaning of the thing could change depending on how the calling code uses those methods.

For example, a mutable Rectangle client may expect to use setWidth and setHeight, which doesn't make sense for the Square subclass.

In a "Normalised class hierarchy" there is only one concrete implementation of every method. When you ask "How does this class do X?" you can know that either the implementation will be in the class, or it will be abstract and implemented in one subclass.

09-i-cant-get-this-class-into-a-test-harness.md

I can't get this into a test harness (p105)

This chapter has some practical advice for getting existing code under test. The global variable exercise below sounds like a good way to explore an unfamiliar codebase or practice refactoring.

Construction tests

This is where you don't assert anything. Just try and construct the object

You will likely need to deal with irritating parameters, for example something that connects to a server and adds delay.

Some techniques:

• Extract an interface and create a fake version of the dependency
• Try just passing null and see if the parameter is actually used
• The null object pattern
• Use subclass and override method to break dependencies

Parameterize constructor

There can also be "hidden" dependencies lurking in constructors. In this case parameterize the constructor:

1. Rename the old constructor to something new
2. Parameterise it
3. Create a new constructor with the the old interface that delegates to the renamed method

Superceded instance variables

Another option for bypassing unwanted dependencies is to swap in a dependency after constructing the object (via setters). You can use this to sense behaviour in tests. Be careful of resource management problems though.

Introduce static setter

Global dependencies are a barrier to reuse because you can't just pull a class out of it's home, and compile and test it independently.

For example, class depending on PermitRespository singleton object - which needs to be intialised with permits.

If you add static setTestingInstance and/or resetForTesting to a singleton, then you can setup the instance in your test setup and teardown, so that it's in a known state at the start of every test, and state doesn't leak between tests. You break the singleton property in exchange for being to instantiate it directly in tests.

If the singleton class has undesirable behaviour, can subclass and override, or extract an interface.

Exercise

• Pick a global variable in your application and see where it's used
• How would you get that object to the objects that need it if it couldn't be a global variable?
• How would we refactor the application?
• Are there responsibilities that we can separate out of sets of classes to decrease the scope of the global?

(If it really is used all over the place, then the application is lacking structure/layering)

Onion parameters

Another reason why it can be hard to construct objects is when you have a whole graph of dependencies to instantiate. But you shouldn't have to instantiate all of this. Look at what is actually used by the class and extract an interface/implementor, or subclass and override methods for the dependencies.

10-i-cant-run-this-method-in-a-test-harness.md

I can't run this method in a test harness (p137)

This is a continuation of the previous chapter. A lot of it was quite programming language specific and I didn't get as much out of this chapter.

Access problems

Either test through the public interface or make the private method public.

A hack for subverting access protection is to make the private method protected, and then create a public accessor in a subclass.

Side effect problems

Command/Query seperation: "A method should be a command or a query, not both"

1. Extract command/query methods
2. Identify groups of methods that can be moved to new classes