Refactoring the code in legacy software is an opportunity to restructure and improve the code source while keeping the same features. Mastering refactoring techniques can save time than throwing the code away and building it from scratch. Refactoring of code becomes strategic when you need to perform modifications to a legacy code base, especially when it lacks unit tests to ensure you won’t introduce regressions. This post is a short journey toward 8 refactoring techniques that would ease your work as a developer.
They stem from a webinar on refactoring organized with our partner Arolla, a French company expert in Software Craftsmanship practices (replay is here, but French only).
8 tips for refactoring your code with no pain
Refactoring Tip #1: Give more meaningful names to code elements
Let’s start with a simple refactoring technique, the renaming. A common guideline in software engineering is to name your variables, functions, and classes so that we can understand their intent and their business value. Good naming will drastically improve the understandability of a piece of code. Assume now you cope with the following code in a class called TennisGame:
public String getScore() {
String s;
if (p1 < 4 && p2 < 4 && !(p1 + p2 == 6)) {
String[] p = new String[]{"Love", "Fifteen", "Thirty", "Forty"};
s = p[p1];
return (p1 == p2) ? s + "-All" : s + "-" + p[p2];
} else {
if (p1 == p2)
return "Deuce";
s = p1 > p2 ? p1N : p2N;
return ((p1-p2)*(p1-p2) == 1) ? "Advantage " + s : "Win for " + s;
}
}
Not an easy one, right? If you struggle a little bit, you’ll eventually figure out the s variable, which has a key role in this function, is used to store the displayed score of the tennis game. Again, with your IDE, a quick win is to perform a rename operation:
public String getScore() {
String displayScore;
if (p1 < 4 && p2 < 4 && !(p1 + p2 == 6)) {
String[] p = new String[]{"Love", "Fifteen", "Thirty", "Forty"};
displayScore = p[p1];
return (p1 == p2) ? displayScore + "-All" : displayScore + "-" + p[p2];
} else {
if (p1 == p2)
return "Deuce";
displayScore = p1 > p2 ? p1N : p2N;
return ((p1-p2)*(p1-p2) == 1) ? "Advantage " + displayScore : "Win for " + displayScore;
}
}
At a glance, we quickly understand now the intent of this variable.
Refactoring Tip #2: Extract loop content in a separate method
Consider the following code (from the Gilded Rose kata):
public void updateQuality() {
for (Item item : items) {
if (!item.name.equals("Aged Brie")
&& !item.name.equals("Backstage passes to a TAFKAL80ETC concert")) {
if (item.quality > 0) {
if (!item.name.equals("Sulfuras, Hand of Ragnaros")) {
item.quality = item.quality - 1;
}
}
} else {
if (item.quality < 50) {
item.quality = item.quality + 1;
// ... lots of code
}
}
// ... more code
}
}
This code has several indentation levels, making it more difficult to read and understand. One quick operation supported by all main IDEs is the Extract Method feature. Give a meaningful name, and here you go!
public void updateQuality() {
for (Item item : items) {
//The content has been extracted into a separate method
updateQuality(item);
}
}
private void updateQuality(Item item) {
if (!item.name.equals("Aged Brie")
&& !item.name.equals("Backstage passes to a TAFKAL80ETC concert")) {
if (item.quality > 0) {
if (!item.name.equals("Sulfuras, Hand of Ragnaros")) {
item.quality = item.quality - 1;
}
}
} else {
if (item.quality < 50) {
item.quality = item.quality + 1;
// ... lots of code
}
}
// ... more code
}
We now have two methods with separate responsibilities (browse the collection and compute each item) and a removed indentation level. Extract method operations are common when refactoring the code.
Refactoring Tip #3: Separate boolean conditions in nested if
This one is not necessarily valid in all contexts, but in some cases, it can help make the code more readable and help understand the code's decision tree. You should see this practice as an intermediate step during your refactoring. Consider the following code:
public void updateQuality() {
for (Item item : items) {
if (!item.name.equals("Aged Brie")
&& !item.name.equals("Backstage passes to a TAFKAL80ETC concert")) {
if (item.quality > 0) {
if (!item.name.equals("Sulfuras, Hand of Ragnaros")) {
item.quality = item.quality - 1;
}
}
} else {
// ... else branch
}
//more code...
}
}
If we burst the conditions in the if statement, we obtain this result:
public void updateQuality() {
for (Item item : items) {
if (!item.name.equals("Aged Brie")) {
if (!item.name.equals("Backstage passes to a TAFKAL80ETC concert")) {
if (item.quality > 0) {
if (!item.name.equals("Sulfuras, Hand of Ragnaros")) {
item.quality = item.quality - 1;
}
}
} else {
// ... else branch
}
} else {
// ... else branch
}
//more code...
}
}
See the intent? You can observe that code in …else branch is indeed duplicated, but you may agree that the conditions are easier to read now. Once you’re more confident with this code, for instance, if you manage to write unit tests that cover the different cases, you’ll be able to make new changes in your code.
Refactoring Tip #4: Rewrite negative conditions
Similar to Tip #3, this is an intermediate step. Too many conditions can increase the mental load required to understand a code. A piece of advice can be to remove the negations, which means turning the following code:
if (!item.name.equals("Sulfuras, Hand of Ragnaros")) {
item.sellIn = item.sellIn - 1;
}
Into this one:
if (item.name.equals("Sulfuras, Hand of Ragnaros")) {
} else {
item.sellIn = item.sellIn - 1;
}
Even though the first condition has no instruction, it still improves reading this code.
Refactoring Tip #5: Encapsulate a static dependency
This case commonly happens with the usage of the Singleton pattern. There are 2 issues with a singleton: 1) you can’t override it 2) its shared state can change from different unit tests execution, making them not reproducible depending on their execution order. The idea here is to extract the call to the dependency in a method so that we can override the default behavior in a sub-class during our unit tests.
As an example here:
public List<Trip> getTripsByUser(User user) throws UserNotLoggedInException {
List<Trip> tripList = new ArrayList<Trip>();
//The following line calls the static dependency
User loggedUser = UserSession.getInstance().getLoggedUser();
boolean isFriend = false;
if (loggedUser != null) {
for (User friend : user.getFriends()) {
if (friend.equals(loggedUser)) {
isFriend = true;
break;
}
}
if (isFriend) {
tripList = TripDAO.findTripsByUser(user);
}
return tripList;
} else {
throw new UserNotLoggedInException();
}
}
This can be turned into the following:
public List<Trip> getTripsByUser(User user) throws UserNotLoggedInException {
List<Trip> tripList = new ArrayList<Trip>();
//We don't directly depend upon a static dependency now.
User loggedUser = getLoggedUser();
boolean isFriend = false;
if (loggedUser != null) {
for (User friend : user.getFriends()) {
if (friend.equals(loggedUser)) {
isFriend = true;
break;
}
}
if (isFriend) {
tripList = TripDAO.findTripsByUser(user);
}
return tripList;
} else {
throw new UserNotLoggedInException();
}
}
User getLoggedUser() {
return UserSession.getInstance().getLoggedUser();
}
The getLoggedUser method will then be overridden if needed in our tests suite.
Refactoring Tip #6: Return early to reduce complexity
A method containing multiple if conditions and exits (return or exceptions thrown) might have a complex decision tree with several indentation levels. The suggestion for refactoring the code here is to turn conditions into their negative form to leave the method as soon as possible. Think as a Fail fast approach. Here is below an illustration with a method containing four exits points:
public decimal Convert(decimal amount, string sourceCurrency, string targetCurrency) {
if (currencyVerifier.Verify(sourceCurrency))
{
if (currencyVerifier.Verify(targetCurrency))
{
if(amount < 0)
{
throw new InvalidOperationException(); //Exit 1
}
decimal conversionRate = _rates.GetRateOf(sourceCurrency, targetCurrency);
if (sourceCurrency.Equals(targetCurrency))
{
return amount; //Exit 2
}
logger.Log(DateTime.Now, sourceCurrency, targetCurrency, conversionRate);
var convertedValue = amount * conversionRate;
return convertedValue; //Exit 3
}
}
throw new InvalidOperationException(); //Exit 4
}
We can turn it into the following code that contains four flattened if conditions, reducing the cognitive complexity of understanding this code:
public decimal Convert(decimal amount, string sourceCurrency, string targetCurrency) {
if (!currencyVerifier.Verify(sourceCurrency))
{
throw new InvalidOperationException();
}
if (!currencyVerifier.Verify(targetCurrency))
{
throw new InvalidOperationException();
}
if (amount < 0)
{
throw new InvalidOperationException();
}
decimal conversionRate = _rates.GetRateOf(sourceCurrency, targetCurrency);
if (sourceCurrency.Equals(targetCurrency))
{
return amount;
}
logger.Log(DateTime.Now, sourceCurrency, targetCurrency, conversionRate);
var convertedValue = amount * conversionRate;
return convertedValue;
}
}
You might see this is counter-intuitive if we consider Tip #4. Remember that the most important thing is to remain pragmatic, depending on the context of your code. Refactoring is about improving the maintainability of the source code, so you’re likely to go through different intermediate steps; trust yourself to choose the one that helps you the most.
Refactoring Tip #7: Declare variables at the closest locations of their usage
An easy trick to improve the readability of a source code is to make sure you don’t declare a variable and use it several lines after. Reading code that doesn’t follow this practice lets us think you give irrelevant information, then do something else, and finally, use the previous information since you need it now.
In this example, there’s no need to declare conversionRate so early:
decimal conversionRate = _rates.GetRateOf(sourceCurrency, targetCurrency);
if (sourceCurrency.Equals(targetCurrency))
{
return amount;
}
logger.Log(DateTime.Now, sourceCurrency, targetCurrency, conversionRate);
var convertedValue = amount * conversionRate;
So while I’m reading the first condition, I’m confused about the role of this variable. Instead, declare it just before using it:
if (sourceCurrency.Equals(targetCurrency))
{
return amount;
}
decimal conversionRate = _rates.GetRateOf(sourceCurrency, targetCurrency);
logger.Log(DateTime.Now, sourceCurrency, targetCurrency, conversionRate);
var convertedValue = amount * conversionRate;
return convertedValue;
The full story is easier to read, and you can keep refactoring the code.
Refactoring Tip #8: Encapsulate primitives types into business types
Business types make it easier to express intention and logic while ensuring validation. In the following code, we need to make assumptions about what are the three input parameters (despite the clear naming):
public decimal Convert(decimal amount, string sourceCurrency, string targetCurrency)
{
if (amount < 0)
{
throw new InvalidOperationException();
}
//...
}
But you can see that the business logic with the amount < 0 condition is scattered in the code and should not be there. Instead, we can suggest the following code, introducing the Amount and Currency classes, which allows encapsulating the previous condition into a method isNegative().
public decimal Convert(Amount amount, Currency sourceCurrency, Currency targetCurrency)
{
if (amount.isNegative())
{
throw new InvalidOperationException();
}
//...
}
Another approach would be to call amount.isLowerThan(0), but that depends on your business ;)
Want to promote your refactoring practices?
In your context, there are certainly more best coding practices you’d like to share and gather with your team, not only about refactoring but also about architecture, security, a programming language, and even more.
Packmind integrates with your IDE to help capture your source code modifications that illustrate a best practice to follow or not. Then you can share your expertise with Packmind and review it during a dedicated workshop with your team.
When refactoring your code, you can value your source code operations and make your expertise available for developers in your project and organization.
This is an example of what a best practice looks like in Packmind.
Ready to refactor the code you’re working on?
We bundled these practices into a catalog available on the public Hub of best coding practices powered by Packmind.
Finally, if you are looking for more refactoring techniques, you can check the dedicated section on the excellent Refactoring.guru website; more than 50 were available when we wrote this post.
Top comments (0)