Executing many automated refactorings in a row is a powerful way to improve your code quickly. The advantage of this approach over manual refactoring is that it is less likely to introduce bugs and that it can often be faster with the right keyboard shortcuts. However, it is a bit of an art to chain refactorings, as it can involve unintuitive actions to enable further steps.
This blog post shows an example of how to simplify a small JavaScript function in a series of 12 automated refactorings without changing its behavior. I'll be using Visual Studio Code and the P42 JavaScript Assistant refactoring extension.
💡 This blog post uses P42 JavaScript Assistant v1.55
Initially, the function (from this blog post) looks at follows:
const lineChecker = (line, isFirstLine) => {
let document = ``;
if (line !== "" && isFirstLine) {
document += `<h1>${line}</h1>`;
} else if (line !== "" && !isFirstLine) {
document += `<p>${line}</p>`;
} else if (line === "") {
document += "<br />";
}
return document;
};
After refactoring, the function is much shorter and easier to comprehend:
const lineChecker = (line, isFirstLine) => {
if (line === "") {
return `<br />`
}
return isFirstLine ? `<h1>${line}</h1>` : `<p>${line}</p>`;
};
Here are the steps that I took to refactor the function:
Simplify Control Flow and Remove Variable
The first refactorings eliminate the document variable and simplify the control flow. This change makes it easier to reason about the function because there is less state (i.e., no document variable) and several execution paths return early.
-
Pull out the
+
from the+=
assignments into regular string concatenation. This step enables the introduction of early return statements in the next step. -
Replace re-assigning the
document
variable with early return statements. This step simplifies the control flow and enables inlining thedocument
variable. -
Inline the
document
variable. This step removes an unnecessary variable and enables the removal of the empty string literals in the next step. - Remove empty string literals by merging them into the templates.
After applying these steps, the function looks as follows:
const lineChecker = (line, isFirstLine) => {
if (line !== "" && isFirstLine) {
return `<h1>${line}</h1>`;
} else if (line !== "" && !isFirstLine) {
return `<p>${line}</p>`;
} else if (line === "") {
return `<br />`;
}
return ``;
};
Simplify Conditions and Remove Code
The next goals are to simplify the conditions in the if statements and to remove dead or unnecessary code. This change further reduces the complexity of the function and makes it easier to comprehend because there is less code and the conditions are simpler.
- Separate
isFirstLine
condition into nested if statement. -
Pull up negation from
!==
. These two steps prepare the removal of the redundant else-if condition. -
Remove redundant condition on else-if because it is always true. After removing the redundant else-if condition, it becomes clear that the final
return
statement is unreachable. - Remove unreachable code. Unreachable code is useless and consumes some of our attention without benefit. It is almost always better to remove it.
-
Push negation back into
===
. This refactoring reverts a previous step that was temporarily necessary to enable further refactorings. -
Invert
!==
condition and merge nested if. The resultingline === ""
condition is easier to understand because there is no negation. Even better, it enables lifting the inner if statement into an else-if sequence and indicates that the empty line handling might be a special case.
After applying these steps, the function looks as follows:
const lineChecker = (line, isFirstLine) => {
if (line === "") {
return `<br />`;
} else if (isFirstLine) {
return `<h1>${line}</h1>`;
} else {
return `<p>${line}</p>`;
}
};
Improve Readability
The last set of refactorings aims to improve the readability by moving the special case line === ''
into a guard clause and using a conditional return expression.
- Convert
line === ''
condition into guard clause. - Simplify return with conditional expression.
- Format, e.g., with Prettier on save.
Here is the final result:
const lineChecker = (line, isFirstLine) => {
if (line === "") {
return `<br />`
}
return isFirstLine ? `<h1>${line}</h1>` : `<p>${line}</p>`;
};
Additional Considerations
This blog post shows how to use automated refactorings to simplify a JavaScript function without changing its behavior. In practice, there are many additional considerations:
Automated Test Coverage
Automated testing is essential to have confidence that the refactoring did not inadvertently change the code's behavior. It is particularly crucial when there are error-prone manual refactoring steps. When there is insufficient test coverage, it is critical to add tests before refactoring code.Uncovering Potential Bugs
Simpler code can uncover potential bugs that you can investigate after the refactoring is completed. In the example here, a<br />
is being returned from the function even whenisFirstLine
istrue
, which might not be the intended behavior.Other Refactorings
There are many ways to refactor the function from this blog post. I've focussed on simplifying the function, but renaming or even decomposing it are other possibilities. Check out the post "How would you refactor this JS function?" for more.
I hope this post gave you an idea of how to sequence automated refactoring steps to achieve a more significant refactoring change.
Top comments (1)
As always, I would much prefer the form
Or alternatively
Early return for anything other than errors just makes the code harder to follow and should be avoided whenever it's not really necessary, which is often a sign that a function does too much.