Note: See canonical post at my blog
So your engineering team is convinced that you need to make some drastic changes. The direction of future development needs to improve. Things can't stay as they are. Management is also convinced that the product needs to move in a new direction. What's next?
Before doing any actual changes or refactoring to your product, planning a refactor is your next step. In other words, you need a game plan. I'll also discuss some refactoring tips for you to get started!
Does Refactoring Mean a Rewrite?
One comment I've seen come up on Reddit about this series (quite a bit...) is the accusation that I'm suggesting you ought to rewrite your entire codebase. I've never said that. Nor have I ever implied that you should. Really, you probably shouldn't.
Trust
As a refresher, in his book Working Effectively With Legacy Code, Michael states:
To me, legacy code is simply code without tests.
Why does Michael care so much about testing your code from the inside? (i.e. not by having people test your website over and over - which is really expensive btw). There's a simple question that can answer this:
If you were to change a core feature of your product in a non-trivial way, would you feel confident about making that change? Would you trust the system to still act exactly as it did?
If you don't have any testing, then how can you be confident? How can you trust your system?
Having tests in place is like doing acrobatics in the air with a safety net vs. not having a safety net. Ouch!
What Are Your Goals?
Your first goal should be to start implementing unit tests on your code. This is foundational work. You need to be able to change your code and have confidence that it still works.
Again:
Your first goal should be to implement code-based testing. You cannot refactor your system with confidence unless you have a safety net.
After this, your goals may vary. If you have completed Step 1 and Step 2 then you should have a solid list of what needs to change.
What I would suggest at this point is having a formal discussion (with a formal outcome/document) that answers the following question:
What do we want our system to look like in 1 year? In 2 years?
Maybe we should be using a new technology stack like ASP .NET Core? Maybe our current code architecture does not allow us to re-use our business rules in other platforms (web vs. mobile API vs. desktop app)? This would imply that we need to consolidate our business logic and rules. (P.s. None of these cases require a re-write)
Dealing With Dependencies
The number one obstacle that (most likely) prevents you from creating isolated unit tests and isolating your business rules and entities are dependencies.
Once you start, you find that you start telling yourself:
Well, in order to test [thing 1] I now need to have an instance of [thing 2]. But, [thing 2] needs an instance of [thing 3].
"Thing 1" might be an entity you want to test - let's say, a Report entity (which models some tabular data).
Now, imagine that "Thing 2" is another class - LinkGenerator (which generates links for the report).
LinkGenerator needs access to "Thing 3", which is, the HttpSession.
If you want to unit test the Report entity, you need:
- an instance of
LinkGeneratorwhich needs... - an instance of
HttpSession
Uh Oh. How can you unit test when you need HttpSession? Unit tests don't run off a web server! (Well, they shouldn't...)
Sorry to say (you already know...), it's going to take some work. You need to break the chain of dependencies.
Fortunately for us, that's one of the primary goals of refactoring. Others have already done the hard lifting for us.
Dependency Breaking And Refactoring Tips
Let's look at a couple dependency breaking refactoring tips.
1. Turn Globals Into Interfaces. Then Inject Them.
The title says it all. Sticking with our little example, imagine the LinkGenerator has the following method (pseudo-ish code).
public string GenerateLink()
{
// ... some kind of processing
var someValue = HttpSession["SomeKey"];
// ... more processing
var someOtherValue = HttpSession["SomeOtherKey"];
return link;
}
We can't test this method because it references the HttpSession object that only exists in a web application. We don't want our models or business entities to know about the web (this is in line with our goal of isolating business entities from the presentation of our data).
By injecting an interface instead, we can remove the dependency on the actual HttpSession.
public string GenerateLink(IHttpSessionAccessor session)
{
// ... some kind of processing
var someValue = session.GetValue("SomeKey");
// ... more processing
var someOtherValue = session.GetValue("SomeOtherKey");
return link;
}
I'm sure you can imagine what the interface definition would look like. The concrete class might look something like this:
public class HttpSessionAccessor : IHttpSessionAccessor
{
private readonly HttpSession _session;
public HttpSessionAccessor(HttpSession session)
{
this._session = session;
}
// You could be fancy and use generics?
public object GetValue(string key)
{
return this._session[key];
}
}
Now, we can do something like this in our testing code:
IHttpSessionAccessor session = new Mock<IHttpSessionAccessor>();
// Implement the mock
// Or just assign "session" with a dummy implementation of IHttpSessionAccessor.
LinkGenerator generator = new LinkGenerator();
string link = generator.GenerateLink(session);
// Assert ...
Now we can build tests around the LinkGenerator and have confidence that:
- It actually works the way we expect it to work
- Any breaking changes will be caught (and fixed).
2. Adapt Parameter
Imagine our code above was originally this:
public string GenerateLink(HttpSession session){
// ... some kind of processing
var someValue = session["SomeKey"];
// ... more processing
var someOtherValue = session["SomeOtherKey"];
return link;
}
What's wrong? We have the same issue as above. We still need an instance of HttpSession. Which means... we need a web server to be running. Bad.
To solve this, just do the same thing as #1. Turn the parameter into an interface and access the interface instead of the actual implementation (HttpSession).
3. Extract Method
You are probably familiar with this technique. If you have a section of code that is doing multiple chunks or has multiple responsibilities, then you need to break them up. Take a chunk of code that does one thing, and create a new method out of it. Avoid references to global state (like HttpSession) so that you can unit test your new methods.
Good indicators of where to break up your code are:
- Comments saying "Do this", then "Do this next thing" inside the same code block are usually each a candidate for extraction.
- C# regions are a really good indicator that you need to split this code up!
Conclusion
The primary areas you need to focus on are:
- Building a game plan describing where you want your system to be in 1 year
- Making your software trustable
- Being able to have confidence after changes are made to the code
Dependencies will need to be broken. But this ultimately leads you to a place where:
- Your code is testable
- Your overall design is (overall) better
- You can trust the system/software after changes
Thanks for reading :) Let me know what you think in the comments. Have you ever had to go through this process personally? Let me know!
Keep In Touch
Don't forget to connect with me on twitter or LinkedIn!
Navigating Your Software Development Career
An e-mail newsletter where I'll answer subscriber questions and offer advice around topics like:
✔ What are the general stages of a software developer?
✔ How do I know which stage I'm at? How do I get to the next stage?
✔ What is a tech leader and how do I become one?
Sound interesting? Join the community!
Top comments (6)
Nice summary of the core approaches for safe refactoring. Dependency injection is such a huge part of that. It's almost like adding a layer of abstraction is a core method of solving problems...
I'm glad you brought up Working Effectively With Legacy Code and Michael's point about trust. We've done a few book club groups on his book and we've found it to be quite useful. For those that are reading this article but haven't read the book I really recommend it. Not all of the chapters may apply to you (the C/C++ macro sections aren't going to help much for the JS crowd) but thankfully the more specialized partsare well defined so it should be easy for you to figure out what chapters to skip.
Thanks for the feedback! I really enjoy that book. It covers so much ground and is super practical.
Great article.
You asked:
and said:
I agree with that. I want to give a link to Greg Young's presentation. Greg said multiple times that during code writing we should thinking about deleting it. I don't wanna to say too much. I recommend it to everyone because Greg shows a whole new perspective :)
Greg Young - The art of destroying software
Thanks! I'll have to take a look at that sometime. Greg Young usually has very insightful things to say.
Great topic and great conclussions. Unit tests should be mandatory in any company!
Thanks Juan!