Refactoring: How to improve legacy code one step at a time. Starting with "if statements".

This is the first post of a series of posts focusing on learning how to methodically improve legacy code, providing actionable steps for detecting bad codes, improving those code with small easy to digest examples.

What is refactoring?

Refactoring can be considered as making changes to code that is focused on improving its readability and maintainability while keeping its functionality the same.

Changing code for performance improvement can also be considered refactoring in certain cases but during the series "refactoring" will be used solely in the context of increasing code quality from a maintainability and readability perspective.

What is bad code?

Any code that is hard to understand, modify or you simply go "The hell is this!" is bad code. (Sorry for the oversimplified explanation here)

Why is refactoring important?

Well refactoring code is important because every line of bad code is black hole for sucking a developer's time and energy. Both of which developers have a limited amount of. IF it takes 6 hours to figure out what a code is doing than its 6 hours less of actually making the code do what you want it to do. Thus, it's always important to maintain a shiny code base that's so easy to understand that it feels like the person who wrote it is explaining to you himself.

In this series we will learn how to do just that (well not that good, but you get what I mean) and improve existing legacy code to point where its no longer at a level of "What the hell is this!" the next time you (or someone else) reads it, instead its at a level of "Okay I understand this" right off the bat.

So, let's start with the simplest part we can improve, the "If statements".

Improving If statements

Whether it's the classic if-else ladder or a simple if statements it's a given that you'll be using it a lot whether you're an intern or a senior dev. Since its written frequently it's also read a lot too. Often due to its perceived simplicity, it's easy to overlook its quality while writing. So, well start off with a common technique of improving if statements or conditionals in general in a codebase.

Decomposing conditionals

Let's look at the below code snippet where we have to calculate the weekly salary of an employee. Where the salary per hour differs for employees with different experience and how many hours employee has worked in contrast to his mandatory work hours.

Let's look at the bad way to write the code or our starting point first. We'll look at a function that is used to return the per hour salary of an employee.

let employee = {
    "Name" : "Congkey",
    "Experience" : 5,
    "Hours worked" : 48,
    "Mandatory Work Hour": 40
};

function salaryPerHour(employee){
    // Unclear about the purpose of the conditions
    if (employee["Experience"] > 5)
        return 20 * employee["Hours worked"] / employee["Mandatory Work Hour"]; // return statements don't indicate what they are. 
    else if (employee["Experience"] < 5 && employee["Experience"] > 2) 
        return 10 * employee["Hours worked"] - employee["Mandatory Work Hour"];
    else if (employee["Experience"] < 2)
        return 5;
}

Here we cant really tell why the conditions exist or what is the purpose of the return values are. This can be improved by wrapping the conditions in functions with self-explanatory names aka decomposing conditions.

For example it can be improved at first as below:

function salaryPerHour(employee){
    // return statements don't indicate what they are.
    if (isSenior(employee))
        return 20 * employee["Hours worked"] / employee["Mandatory Work Hour"]; 
    else if (isMidLevel(employee))
        return 10 * (employee["Hours worked"] / employee["Mandatory Work Hour"] % 3);
    else if (isJunior(employee))
        return 5 * ( employee["Hours worked"] / employee["Mandatory Work Hour"] % 2);
}

// extracted conditions
function isSenior(employee) {
    return employee["Experience"] > 5;
}

function isMidLevel(employee) {
    return employee["Experience"] < 5 && employee["Experience"] > 2;
}

function isJunior(employee) {
    return employee["Experience"] < 2;
}

At this stage the conditions inside SalaryPerHour is cleaner as we can tell what each condition is for. But it's yet unclear what the return values actually are for each condition or why they are so. We can specify it by extracting the return values into their own functions.

Example:

function salaryPerHour(employee){
    if (isSenior(employee))
        return seniorLevelHourlyRate(); 
    else if (isMidLevel(employee))
        return midLevelHourlyRate();
    else if (isJunior(employee))
        return JuniorLevelHourlyRate();
}

// extracted conditions
function isSenior(employee) {
    return employee["Experience"] > 5;
}

function isMidLevel(employee) {
    return employee["Experience"] < 5 && employee["Experience"] > 2;
}

function isJunior(employee) {
    return employee["Experience"] < 2;
}

function seniorHourlyRate(employee) {
    return 20 * employee["Hours worked"] / employee["Mandatory Work Hour"];
}

function midLevelHourlyRate() {
    return 10 * (employee["Hours worked"] / employee["Mandatory Work Hour"] % 3);
}

function juniorLevelHourlyRate() {
    return 5 * ( employee["Hours worked"] / employee["Mandatory Work Hour"] % 2);
}

Now we can also tell what each condition is for and what the conditions are returning. This will make changing the function much easier as each function is well separated and clearly self-explanatory. For OOP we can do the same by decomposing conditions inside methods. Smaller functions also help with testing the code and minimizing chances of error. This is also an application of Single responsibility principle of Object-Oriented Programming.

The next post of this series will contain more techniques on improving if statements.

[Side note: I am starting this series as a summarized lessons of what I have learned on refactoring legacy code. I am by no means a great expert, what I say is context specific and might not be usable for your specific case. There are incredible software engineers who have written books, research paper or has tutorials on this topic. So if you find something more to improve upon what I have written leave a comment and I'll make changes accordingly. And if you have enjoyed or found the post helpful still leave feedback as your appreciation and knowing that my work is helping someone is the greatest motivator for me.]