DEV Community

KiminLee
KiminLee

Posted on

Refactoring findBreakURL to improve maintainability!

Before Start

While I am participating "2020 hacktoberfest", I realized that some of the projects on github are not really easy to get start as a new contributor for them. The reason is their codes usually written by one or two contributors and didn't really think about maintainability and let others join in the project. Most of the my projects are also done by me, so to be honest, I didn't really care about the maintainability and conventions as well. findBreakURL is also one of them. However, being a open source developer is not only improving my coding skills but also learn how to interactively work with others and effectively maintain the existing codes.

So, this week, I have decided to improve the code style of findBreakURL

What should be changed?

So far, the program has 4 different .js files.

  • fetch.js : With axios, we will do 'header fetch' to check status code
  • json.js : This file will make a json style output
  • yargs.js : This file handle the arguments and options and read directory to find files
  • index.js : This is the entry for the program, will read files and following functions

Wait, What? index.js will read files? and yargs.js read directory to find files?
This looks not so consistency and maybe a little bit irrelevant for their desired functions. Maybe we can split the functions to another files.

let's start with git checkout -b refactoring

Step 0 : Make a fileRead.js

It will be better if we extract reading a file/directory function from index.js and yargs.js and make a separate file for the change.

Step 1 : Make a fileRead() function

In index.js, fileData is extracted.

...
try {
        fileData = fs.readFileSync(`${__dirname}\\${file}`, {
            encoding: "utf-8",
        });

    // wrong file name
    } catch (error) {
        console.log(file + " is a WRONG file name");
        return process.exit(1);
    }
...
Enter fullscreen mode Exit fullscreen mode

Maybe we can make this part as a fileRead() function:

const fileRead = (file) => {
    let fileData = null;

    try {
        fileData = fs.readFileSync(`${__dirname}\\${file}`, {
            encoding: "utf-8",
        ...
    return fileData
Enter fullscreen mode Exit fullscreen mode

After change, commit this change.
git add index.js + git commit split index.js to index.js and fileRead.js

Step 2 : Make a fileDirectory() function

In the yargs.js, files are declared globally. And all conditions are not inside a function. It looks better if the variable is only used inside a function and the conditions are also in the function.

Before:

let files = [];
    if (yargs.argv._.length === 1 && yargs.argv._[0] === "start") {

      // decide the option if it is -f or -a
      if (yargs.argv.a && typeof yargs.argv.f !== "string") {
        const tmpFiles = fs.readdirSync(__dirname, { encoding: "utf-8" });

        // if -a, store all files into the files variable
        files = tmpFiles.filter((file) => {
          return file.toLowerCase().endsWith(".html");
        });

      } else if (typeof yargs.argv.f === "string") {
        files = [...yargs.argv.f.split(",")];
      } else {
        console.log(chalk.yellow("Please enter filenames or -a following -f"))
      }
      return files;
    } else {
      console.log(chalk.yellow("Wrong argument: use [start]"));
    }
Enter fullscreen mode Exit fullscreen mode

After:

const readDirectory = (yargs) => {
    let files = [];
    if (yargs.argv._.length === 1 && yargs.argv._[0] === "start") {

      ...
Enter fullscreen mode Exit fullscreen mode

Commit the changes as extract fileDirectory(), put that into fileRead.js

Step 4 : Split index.js and yargs.js

Now let's move those functions to fileRead.js:

const fs = require("fs");
const chalk = require('chalk')

const fileRead = (file) => {
    ...
}

const readDirectory = (yargs) => {
    ...
}

module.exports = { fileRead, readDirectory }
Enter fullscreen mode Exit fullscreen mode

And index.js should be used fileRead and yargs needs 'readDirectory'
index.js:

// Start testing files each
files.map((file) => {
  const fileData = fileRead(file);
...
Enter fullscreen mode Exit fullscreen mode

yargs.js :

...
const files = readDirectory(yargs);

module.exports = {
  files,
  arg: yargs.argv,
};
Enter fullscreen mode Exit fullscreen mode

Step 5 : Squash the commit and merge!

We have currently 4 changes. However, we only need the last finalized version of changes. We can do this using with git rebase master -i
It enable us to squash all the previous changes and make them a one single commit

pick 4710009 split index.js to index.js and fileRead.js
squash 0c01069 extract fileDirectory(), put that into fileRead.js
squash fd8e932 variables name changed
squash f9d85bf squash index.js and package-lock.json
Enter fullscreen mode Exit fullscreen mode

Also, we can change the commit name with git commit --ammend:

commit d740f80e8c0e2b1d4c9d50613e5812016bcf86de (HEAD -> master, origin/master, origin/HEAD, refactoring)
Author: klee214 <klee214@myseneca.ca>
Date:   Fri Oct 23 12:25:19 2020 -0400

    Refactoring findBreakUrl to improve code maintainability

    *extract fileDirectory(), put that into fileRead.js

    *variables name changed

    *split index.js to fileRead.js and index.js
...
Enter fullscreen mode Exit fullscreen mode

More information

Visit and see this

Oldest comments (0)