SOOO! For the last few weeks, with the help of other students in the course, my static site generator program has become very functional, but it has also grown in size and I was definitely in "technical debt". It was high time to refactor the code base.
Sit tight, because I have a lot to cover
Code refactoring
Below are the steps I took to repay my "technical debt"
Changing file paths handling to 'path' module
Most of my operations related to any file path were done using regEx or string functions, for example:
else if(stat.isFile() && filePath.split('.').pop() == "txt") {
createHtmlFiles(filePath, "txt");
//...
const fullFilePath = `${outputPath}/${filePath.match(/([^\/]+$)/g)[0].split('.')[0]}.html`;
They don't look very readable, especially all the regular expressions. After working on others' code, I found that they used 'path' module which I also had used before, I just questioned myself why did I not do that. Below are some of the changes, they turned out to be easier to figure out what are they trying to achieve
else if (stat.isFile() && path.extname(filePath) == ".txt") {
this.createHTMLFile(filePath, ".txt");
}
//...
const fullOutputPath = path.join(this.outputPath, `${path.basename(filePath, fileType)}.html`)
Change variable names
Not all function and variable names made sense. My function createHTMLFiles() only create one file at the time => createHTMLFile(), my variable fullFilePath left me a question which is "hhhm, what fullFilePath, did I mean the full output path. of a file" => fullOutputPath.
More changes were made
-
fileToHtml=>fileToHtmlCreatorbecause this variable is a HtmlCreator object -
paragraphObj=>bodyObjbecause the name was outdated, it was a 'paragraph object' for .txt files but now it's more like a 'html body object'
Extracting a function
I noticed that this piece of code was used twice in a similar way to write html files to the output folder, line 80, line 168
fs.writeFile(fullFilePath, fileToHtml.renderHTML().replace(/<html>/, `<html lang="${option.lang ? option.lang : defaultLang}">`), (err) => {
if(err)
return console.error(`Unable to create file ${fullFilePath}`);
console.log(`${fullFilePath} is created`);
});
I then wrote a separate function that does the same job to reduce duplicate code
writeHTMLFile = (fullOutputPath, fileToHtmlCreator) => {
fs.writeFile(
fullOutputPath,
fileToHtmlCreator
.renderHTML()
.replace(/<html>/, `<html lang="${this.lang}">`),
(err) => {
if (err)
return errorToConsole(`Unable to create file ${fullOutputPath}`);
else console.log("\x1b[36m", `${fullOutputPath} is created`, "\x1b[0m");
}
);
};
Refactoring the way of catching --config option
I was also not satisfied with the way the program handled --config option, so I rewrote most of the code. My approach was to catch the --config option, parse the .json first before other 'options', override the option object and return with error if --input is not specified
if(option.config){
try {
let configData = fs.readFileSync(option.config);
let configOptions = JSON.parse(configData);
for(const [key, value] of Object.entries(configOptions)) {
value || value.length > 0 ? option[`${key}`] = `${value}` : option[`${key}`] = undefined;
}
if(!option.input) {
console.error('\x1B[31m', `error: input <file or directory> is not specified in config file ${option.config}`, '\x1B[0m');
process.exit(-1);
}
} catch(error) {
console.error('\x1B[31m', `Can't read or parse config file ${option.config}\n ${error}`, '\x1B[0m');
process.exit(-1);
}
}
With this change, the number of lines drastically reduced.
Extracting a class
After refactoring the index.js file, I wanted to get rid of global variables so I came to a decision to create a class/module in a new file ssg.js that does all the file processing while index.js takes care of the command line. Therefore, I moved all global variables to be data members of SSG class and all functions into SSG class, I also tweaked them a bit to fit class syntaxes.
It definitely looks clearer now which also makes future debugging easier.
if(option.input) {
var ssg = new SSG(option.input, option.output, option.lang);
ssg.processInput(option.input);
}
Squashing all the commits into one
Each step refactoring the code base was one or more commit, since we usually care about the end result not really about the process, it's better to squash all the commits into one commit with a clear message consists of bullet points of the changes. The steps are:
git rebase main -i
Choosing which commit to be squashed and moving on to changing all commit messages into one
git commit --amend
In the end, I pushed the changes to GitHub, it was quite some work!
Checkout THE COMMIT
Top comments (0)