Working in legacy code makes it clear how important refactoring is to maintainability.
I'll go so far with the importance of Refactoring to say, "If the code isn't refactored, it isn't maintainable."
Clearly I think this is a huge thing.
One of the definitions of "Legacy Code" that I like is "code without tests". This is mostly true. Just having tests doesn't prevent the code from being legacy code. I think of "Legacy Code" as "code you don't want to touch". Tests or not, if it's a hairy bit of code you're not comfortable in - It's Legacy Code.
For me, this is the core of the idea that legacy code is code without tests. Tests are what give us confidence to work in code.
Tests CAN give us code we're able to confidently refactor... but if we don't refactor, it continues to accumulate cruft. We can't get code into a better state without refactoring it.
Pass The Bigger Thing
What's the bigger thing? This idea ties into my thoughts around Trusting Your Code.
If you haven't read that; totally should. ;) It covers some of the mental shift I've had when writing and refactoring code.
Let's get into the idea and then I'll tie it back to trusting your code.
I'll make up a stupid example to demonstrate this.
Let's have a hacky method to take a name and a nickname and string them together.
string Nameify(string firstName, string lastName, string nickName){
return $"{firstname} ""{nickName}"" {lastName}";
}
This method could easily be TDD'd. It encapsulates a behavior and can be used anywhere in the code. Ignoring that it's not in a class...
What's wrong with this? Initially, I'd say nothing. It exists in this state because we need this functionality. It's how we thinking about it.
This method is implemented exactly as we'd think about it. Take the arguments, put them together and return the result.
A test for this would look like
Nameify("first", "last", "nick").Should().Be("first \"nick\" last");
Perfect. Totally works.
Let's use it!
Nameify(user.FirstName, user.LastName, user.PreferredName);
Perfect. Everything works great.
What's to change?
The title gives it away pretty well. All the values we're pulling out are from the user object.
Looking at the Nameify method; nothing stands out about the method that indicates any potential refactors.
string Nameify(string firstName, string lastName, string nickName){
return $"{firstname} ""{nickName}"" {lastName}";
}
The use of the method can stand out. Extracting values to pass into a method is a code smell.
Everything we're doing with Nameify (the hacky example) is from the user object.
The Legacy Code Refactor this post is about, is to stop extracting values.
Don't Extract from The Object
Let's refactor the Nameify method so that we're just passing in the user object.
string Nameify(IUser user){
return $"{user.FirstName} ""{user.PreferredName}"" {user.LastName}";
}
When we pass in the object, this behavior has a feature that suddenly LEAPS out at us. It's all about the user. In a simple case, maybe we can see it without this refactor. This refactor makes it easier to see in the complex cases. It brings the objects with the data we need together so we can see the REAL relationships.
When we can see what types of cohesion exist to create the behavior we want, we can see how we might better refactor the code.
In this case, since it's only using IUser, we can see that the behavior belongs in the IUser interface.
This process is part of how we remove the getters from our code. We find the behaviors they're used for, and find how to pull that into the object itself. Put the behavior with the data.
While this is a pretty simple example, doing this simplifies code a lot.
Trust
If you've read the Trusting Your Code post you might recognize that this as "Trust what you call". We don't need to pull out all the data and hand it off. That's assuming we know more than the method about what it wants to do... Rude.
Trust your methods to the right thing, give them the authoritative source.
Trust what you call by giving it the object that has the information it needs.