8 tips for refactoring the code with no pain
Tags: refactoring, programming, tutorial, productivity
Published: 2023-01-11 08:23:46

**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](https://www.youtube.com/watch?v=XKoak0RLPw8&ab_channel=ProMyze), 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*:

```java
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:

```java
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):

```java
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!

```java
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:

```java
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:

```java
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:

```java
 if (!item.name.equals("Sulfuras, Hand of Ragnaros")) {
     item.sellIn = item.sellIn - 1;
 }
```

Into this one:

```java
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:

```java
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:

```java
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:

```java
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](https://dev.to/promyze/cyclomatic-complexity-and-cognitive-complexity-4c03) of understanding this code: 

```java
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:

```java
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:

```java
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):

```java
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()*.

```java
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](https://packmind.com) 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](https://www.packmind.com/community-of-practice-in-software-engineering-teams/). 

![This is an example of what a best practice looks like in Packmind.](https://www.promyze.com/wp-content/uploads/BestPracticeHub.png)

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](https://bestcodingpractices.dev/catalog/63970877498b45115fb261d5) available on the public Hub of best coding practices powered by [Packmind](https://packmind.com). 

Finally, if you are looking for more refactoring techniques, you can check the dedicated section on the excellent [Refactoring.guru](https://refactoring.guru/refactoring/techniques) website; more than 50 were available when we wrote this post.