Welcome to today’s post.
In today’s post I will go through the topic of code refactoring.
As a developer, you would be aware of the minimum standards you would have to reach when contributing towards a code base that is maintained by other developers. The common language in the software development world is not linguistic, but the development tools and languages that you use to deliver the task, application, component, or tool into the resulting repository.
One common problem that we encounter is the need to refactor.
In this post I will start by explaining what code refactoring is, then go through some useful examples of how to apply some useful refactoring principles to your own code base.
What is Code Refactoring?
Refactoring is the iteration we go through during our development to partition our code into reusable parcels of methods, functions, classes, components, modules, and libraries.
What are some common code refactoring methods?
Common Code Refactoring Methods:
- Excessive use of literals: these should be coded as named constants, to improve readability and to avoid programming errors.
- Duplicated code: identical or similar code exists in more than one location.
- Replace Nested Conditional with Guard Clauses.
- Data clump – When groups of variables are passed around together in various parts of the program within methods and functions.
- Dead code removal
- Separation of large classes (God objects) into smaller classes.
- Concentrated areas of code that are commented and the code is not self-explanatory.
- Excessively long identifiers: in particular, the use of naming conventions to provide disambiguation that should be implicit in the software architecture. Excessively short identifiers: the name of a variable should reflect its function unless the function is obvious.
I will introduce a basic example in TypeScript and go over the first four of the above refactoring methods.
Examples of Code Refactoring
Literal Refactoring
Below we have a validate file input method that can do with some improvement:
fileSelectionChanged(event: Event)
{
const element = event.currentTarget as HTMLInputElement;
this.selFiles = element.files;
let fileList: FileList | null = element.files;
if (fileList) {
for (let itm in fileList)
{
if (itm === 'length')
break;
let item: File = fileList[itm];
this.ValidateInputFile({
FileName: item['name'],
Size: item['size'],
Status: "PENDING",
Uploaded:"NO" });
}
}
}
The use of literals in the code below:
this.ValidateInputFile({
FileName: item['name'],
Size: item['size'],
Status: "PENDING",
Uploaded:"NO" });
The above can be refactored by replacing the literals with static classes containing the literals:
The file upload description literals can be replaced with the following class:
export class FileUploadDescription
{
public static PENDING: string = "PENDING";
public static COMPLETED: string = "COMPLETED";
}
The file upload status literals can be replaced with the following class:
export class UploadStatus
{
public static YES: string = "YES";
public static NO: string = "NO";
}
The resulting code looks cleaner:
let validFile = {
FileName: value.FileName,
Size: value.Size,
Status: FileUploadDescription.PENDING,
Uploaded: UploadStatus.NO
};
Refactoring Code Duplication
Still, the following code has a problem with code duplication:
ValidateInputFile(value: FileUploadedStatus)
{
if (value.Size > this._filelengthLimitation)
{
let invalidFile = {
FileName: value.FileName,
Size: value.Size,
ValidationError: "Maximum File Size Exceeded"
};
this.invalidFiles.push(invalidFile);
}
else
if (!this.isFileTypeAllowed(value.FileName.match(/\.([^\.]+)$/)[1]))
{
let invalidFile = {
FileName: value.FileName,
Size: value.Size,
ValidationError: "Invalid File Type"
};
this.invalidFiles.push(invalidFile);
}
else
if ((value.FileName.match(/\d+/g) != null) &&
(!this.selectedFiles.includes(value.FileName)))
{
this.selectedFiles.push(value.FileName);
let validFile = {
FileName: value.FileName,
Size: value.Size,
Status:"PENDING",
Uploaded:"NO"
};
this.fileListStatus.push(validFile);
}
}
Notice that there are three sections of the method containing the following code pattern:
let invalidFile = { ... };
this.invalidFiles.push(invalidFile);
We can refactor the above into an additional helper method to assign the data structure and add it to the array:
addInvalidFile(fileName: string, size: number,
status: string, uploaded: string, errText: string)
{
let invalidFile = {
FileName: fileName,
Size: size,
ValidationError: errText
};
this.invalidFiles.push(invalidFile)
}
addValidFile(fileName: string, size: number, status: string, uploaded: string)
{
this.selectedFiles.push(fileName);
let validFile = {
FileName: fileName,
Size: size,
Status: status,
Uploaded: status
};
this.fileListStatus.push(validFile);
}
Parameter Refactoring
Looking closely at the two above methods we can spot further improvements.
Notice that both methods share common parametrizations. We can group the following parameters:
fileName: string, size: number, status: string, uploaded: string
into a common structure to hold our parameter values:
export class FileUploadedStatus
{
public FileName: string;
public Size: number;
public Status: string;
public Uploaded: string;
}
From this we can improve our helper methods by refactoring the parameters. This is applying the refactoring principle of Data Clumping:
addInvalidFile(value: FileUploadedStatus, errText: string)
{
let invalidFile = {
FileName: value.FileName,
Size: value.Size,
ValidationError: errText
};
this.invalidFiles.push(invalidFile)
}
addValidFile(value: FileUploadedStatus)
{
this.selectedFiles.push(value.FileName);
let validFile = {
FileName: value.FileName,
Size: value.Size,
Status: FileUploadDescription.PENDING,
Uploaded: UploadStatus.NO
};
this.fileListStatus.push(validFile);
}
We notice that the use of regular expression string checking in our code leads for a further improvement.
We can move these functions into their own shared class:
export class FileNameUtility
{
public static FILENAME_EXTENSION = (text: string) => text.match(/\.([^\.]+)$/)[1];
public static IS_VALID_FILENAME = (text: string) => text.match(/\d+/g) != null;
}
Also noticed the use of hard-coded strings for validation errors and error messages. These can be moved into their own helper class:
export class UploadValidationErrors
{
public static MAX_FILE_SIZE_EXCEEDED: string = "Maximum File Size Exceeded";
public static INVALID_FILE_TYPE: string = "Invalid File Type";
}
After applying these improvements, the resulting code looks as shown:
ValidateInputFile(value: FileUploadedStatus)
{
if (value.Size > this._filelengthLimitation)
{
this.addInvalidFile(value,
UploadValidationErrors.MAX_FILE_SIZE_EXCEEDED);
}
else
if (!this.isFileTypeAllowed(FileNameUtility.FILENAME_EXTENSION(
value.FileName)))
{
this.addInvalidFile(value,
UploadValidationErrors.INVALID_FILE_TYPE);
}
else
if (FileNameUtility.IS_VALID_FILENAME(value.FileName))
{
this.addValidFile(value);
}
}
We have managed to reduce our main function code from 10 lines down to 6 lines of more readable code. The helper methods can also be re-used elsewhere if needed.
Refactoring Code Guards
The next type of refactoring is to introduce code guards.
Consider the following code. It consists of two if .. then conditional blocks:
combineLatest(
[
topBooks$,
topMembers$
]
).subscribe(([topbooks, topmembers]) => {
if (topbooks && topbooks.length > 0)
{
topbooks.forEach(a =>
{
// do something
});
}
if (topmembers && topmembers.length > 0)
{
topmembers.forEach(a =>
{
// do something
});
}
...
);
The problem with the above is that we are checking the code within the block on multiple occasions.
We can implement a code guard to achieve two of the following goals here:
- Prevent excess processing of code within our code block.
- Simplify the code and logic.
- Open our code to further refactoring opportunities.
Once we apply 1 and 2, we have the following improvement where the checking logic is brought up to the beginning of the code block, with the two conditional checks being removed:
combineLatest(
[
topBooks$,
topMembers$
])
.subscribe(([topbooks, topmembers]) => {
if (!topbooks || topbooks.length === 0 ||
!topmembers || topmembers.length === 0)
return;
topbooks.forEach(a =>
{
// do something
});
topmembers.forEach(a =>
{
// do something
});
...
});
With the third goal, the guard condition:
if (!topbooks || topbooks.length === 0 || !topmembers || topmembers.length === 0)
return;
can be refactored into its own logic class function to protect our code block from null objects or empty arrays:
if (this.isNullOrEmpty(topbooks) || this.isNull(topmembers))
return;
We can replace the above condition with helper logic functions for checking value nullity:
isNull(value: any)
{
return !value;
}
And a helper function for checking non- empty lists is shown below:
isNullOrEmpty(value: any[])
{
return !value || value.length === 0;
}
The resulting refactored code with a code guard is as follows:
combineLatest(
[
topBooks$,
topMembers$
]
).subscribe(([topbooks, topmembers]) => {
if (this.isNullOrEmpty(topbooks) || this.isNullOrEmpty(topmembers))
return;
topbooks.forEach(a =>
{
// do something
});
topmembers.forEach(a =>
{
// do something
});
...
});
As we have seen, it is possible to refactor your code before re-testing and checking in by applying several simple principles.
With practice you can apply the principles as you develop instead of at the end of the development iteration.
For more detailed treatment on code refactoring, it is worth reading the seminal work on code refactoring by Martin Flower.
That’s all for today’s post.
I hope you found this post useful and informative.
Andrew Halil is a blogger, author and software developer with expertise of many areas in the information technology industry including full-stack web and native cloud based development, test driven development and Devops.
