Have you ever encountered code that you know something's wrong with, but you can't tell exactly what? You just know it looks bad.
Or maybe you are getting started in a new team or company and you want to make sense of the codebase, or even improve it.
Refactoring is a wonderful way of starting to learn a new codebase. As you refactor and clean up the code, you will be forced to figure out the logic and you will be able to understand the architecture more deeply. One thing to consider is to refactor BEFORE adding any new complex functionality or feature. Doing it after will require you to refactor more and the complexity will only keep increasing.
How To Start the Refactoring Process?
Create Tests in order to understand what is going on.
By creating tests you will have to look for the functions of the file you are refactoring. Doing this will help you see the data coming in and out of the functions. While you write tests, you are also writing documentation. There is no better documentation than tests.
**Look for Code Smells. **
Looking for them combines applying well-established good practices and trusting your intuition (aka experience). Code smells are basically things that make the code too complex, hard to read, and just UGLY.
We have all written ugly code at some point, but dealing with other people's ugly code could be tricky. Also, there are plenty of code-smells lists out there, so I tried to compile the 7 most important ones (for me):
Code-Smells
Here is some code that really needs refactoring. You don't even need to understand what it's doing to know it needs it.
export function calc (movArray) {
let result = 0;
for (const mov of movArray) {
if (mov.dist != null && mov.dist != undefined && typeof mov.dist === "number" && mov.dist > 0) {
if (mov.ds != null && mov.ds != undefined && mov.ds instanceof Date && mov.ds.toString() !== "Invalid Date") {
if (mov.ds.getHours() >= 22 || mov.ds.getHours() <= 6) {
if (mov.ds.getDay() !== 0) {
result += mov.dist * 3.90;
} else {
result += mov.dist * 5;
}
} else {
if (mov.ds.getDay() === 0) {
result += mov.dist * 2.9;
} else {
result += mov.dist * 2.10;
}
}
} else {
return -2;
}
} else {
return -1;
}
}
if (result < 10) {
return 10;
} else {
return result;
}
}
STOP. Before moving along, take a look at the code again and try to use your intuition to decide what should be changed. Now we will look at a couple of common code smells and what to do with them:
1. Strange or confusing names for variables or functions
Rename them.
Make the name make sense for the data it contains or the functionality it has.
This also applies to files, classes, etc.
2. Magic Numbers that aren't self-explanatory
- Extract them into constants and make them variables with names that explain what they are.
4. Dead Code
Code that is commented out and has no functionality anymore.
Delete it. It can always be recovered from the git history if they end up affecting something in a very obscure, unpredictable way (and should also be refactored if this happens).
5. Blank space/lines (for methods/functions)
Remove them for readability.
Different developers have different spacing styles. It is better just to remove it so everyone's code looks somewhat similar.
For those who prefer spaces when coding (dyslexia, reading disability), code your way but then you could use prettier/eslint to format it and remove the blank spaces when you commit.
6. Confusing conditions / Nested Conditions
Use extract methods (methods that return the logic you want to check for) so that conditions are not long
For example, if you have a long condition inside an IF statement, you could extract it to a function that returns it for you.
Long condition:
if (date.getHours() >= OVERNIGHT_START || date.getHours() <= OVERNIGHT_END) {
}
Extracted to a function:
function isOvernight (date) {
return date.getHours() >= OVERNIGHT_START || date.getHours() <= OVERNIGHT_END;
}
if (isOvernight(segment.date)) {
}
Implement return statements to exit early in case of conditions that check if we are NOT going to execute the main logic. Move "else-returns" to the top of the function. These are called guard clauses. They help you exit the algorithm's logic fast so that the program doesn’t get into the “main logic” if it doesn’t need to. This will help remove nested conditions.
Remove nested conditions by consolidating conditions. This means composing conditions.
If you are inside a loop, add a "continue" statement on “if checks” so that the loop can continue onto the next iteration.
Example of nested conditions:
if (isOvernight(segment.date)) {
if (!isSunday(segment.date)) {
fare += segment.distance * OVERNIGHT_FARE;
} else {
fare += segment.distance * OVERNIGHT_SUNDAY_FARE;
}
}
After consolidation/composition && adding continue statements:
if (isOvernight(segment.date) && !isSunday(segment.date)) {
fare += segment.distance * OVERNIGHT_FARE;
continue;
}
if (isOvernight(segment.date) && isSunday(segment.date)) {
fare += segment.distance * OVERNIGHT_SUNDAY_FARE;
continue;
}
- Use a ternary condition if you have an if/else that’s just for doing an assignment
7. Lack of expectation treatment
- Throw errors instead of just returning something or some magic number that is supposed to mean something.
Code after steps 1-7:
const OVERNIGHT_FARE = 3.90;
const SUNDAY_FARE = 2.90;
const OVERNIGHT_SUNDAY_FARE = 5;
const NORMAL_FARE = 2.1;
const OVERNIGHT_START = 22;
const OVERNIGHT_END = 6;
const MIN_FARE = 10;
function isOvernight (date) {
return date.getHours() >= OVERNIGHT_START || date.getHours() <= OVERNIGHT_END;
}
function isSunday (date) {
return date.getDay() === 0;
}
function isValidDistance (distance) {
return distance != null && distance != undefined && typeof distance === "number" && distance > 0;
}
function isValidDate (date) {
return date != null && date != undefined && date instanceof Date && date.toString() !== "Invalid Date";
}
export function calculateRide (segments) {
let fare = 0;
for (const segment of segments) {
if (!isValidDistance(segment.distance)) throw new Error("Invalid Distance");
if (!isValidDate(segment.date)) throw new Error("Invalid Date");
if (isOvernight(segment.date) && !isSunday(segment.date)) {
fare += segment.distance * OVERNIGHT_FARE;
continue;
}
if (isOvernight(segment.date) && isSunday(segment.date)) {
fare += segment.distance * OVERNIGHT_SUNDAY_FARE;
continue;
}
if (isSunday(segment.date)) {
fare += segment.distance * SUNDAY_FARE;
continue;
}
fare += segment.distance * NORMAL_FARE;
}
return (fare > MIN_FARE) ? fare : MIN_FARE;
}
Many Engineers would point out that there are more things that could be improved, like the method being too long, or that the code is still procedural and should be object-oriented. I understand those recommendations, but I am trying to keep my own recommendations as non-dogmatic as possible. Also, let's not forget that overengineering is a thing, and many "recommendations" are a matter of preference.
Conclusion
Refactoring code smells may seem daunting if it is your first time. But having clean, readable code pays dividends in the long run. Don't let technical debt accumulate. Refactoring improves understandability, reduces bugs, and makes adding new features easier. With some practice, you'll learn how to identify and eliminate code smells efficiently. Work on small sections at a time. Over time, you'll develop an intuition for which refactorings provide the most value. The results are absolutely worth the investment - both for you and for other engineers who will work on your projects in the future.
