Refactoring existing code and rebasing
Published: 2021-10-15 02:28:51

## Background
I was looking at my static site generation tool, **[cli-ssg](https://github.com/dhillonks/cli-ssg)** and being an **open source** repository, it had received a couple of commits from **contributors**. Now while the tool definitely worked as expected, I thought that this was a good moment to **refactor** the codebase to improve the **structure** and **maintainability** of the project.

## Refactoring cli-ssg
I started off by creating a `refactoring` branch from `main`. I then spent some time looking in the project to find areas where I could improve the code quality.

When I had initiated this project, I decided to go with a **module based approach** even though at that time it only really supported `.txt` input. This drastically **reduced** the **refactoring** effort as a major portion of the code involving `.html` generation was already in a separate module called `generateHtml.js`.

That being said, `index.js` was looking a bit **messy** after the pull request for the `--config` option was **merged** into main ([#18](https://github.com/dhillonks/cli-ssg/pull/18)). 

The way the options for the input path, output directory, language and stylesheet were being **parsed** and **validated** could definitely use some **improvement** and I decided to **refactor** the following code:

```js
  .check((argv) => {

    //Input
    if(argv.i){

      if(!fs.existsSync(argv.i)){
        throw new Error("Input path must be a file or directory");
      }
    }

    //Output
    else if(argv.o != outputDir){
      if(fs.existsSync(argv.o)){
        if(!fs.lstatSync(argv.o).isDirectory()){
          throw new Error("Output path points to a file. Output directory must be valid")
        }
      }
      else throw new Error("Output directory must be valid");
    }

    //Config
    else if(argv.c){

      if(isJSON(process.argv[3])){

        if(!fs.existsSync(argv.c)){
          throw new Error("JSON file path does not exist");
        }
        var data = JSON.parse(fs.readFileSync(argv.c));

        if(data.input)      argv.i = data.input
        if(data.output)     argv.o = data.output
        if(data.stylesheet) argv.s = data.stylesheet
        if(data.lang)       argv.l = data.lang
        
      }else{
        throw new Error("The passed file should be of JSON format")
      }
    }
    else {
      throw new Error("A config file(.JSON) or an input file is required");
    }
    
    return true;
  })
  .argv;
```

Before I started **refactoring**, I noticed some problems with the code which were missed during code review:
- **No error handling** when reading/parsing the JSON file
- **No validation** for input, output options when they were parsed from JSON
- An **unhandled error** would be thrown if an output directory path, which did not exist, was passed to the tool.

To fix these problems, I decided to **refactor** the **option parsing** and **validation** process into a **new module**. The class `Options` could then be used to store and process options for the tool such as:
```
input: Input file/folder to be processed
output: Output directory
lang: Language attribute for the html element
stylesheet: CSS stylesheet for the website
```
To store and process these, I wrote a **class** called `Options` and stored it in `configOptions.js`.
```js
class Options{
  constructor(input, output, stylesheet, lang){
    this.input = input;
    this.output = output;
    this.stylesheet = stylesheet;
    this.lang = lang;
  }
  static get DEFAULT_OUTPUT(){ return './dist'; }
  static get DEFAULT_LANG(){ return 'en-CA'; }
}
module.exports.Options = Options;
```

Refactoring the config parsing functionality into this new module:
```js
class Options{
   /**
   * Parses options using a file path
   * @param {string} filePath 
   * @return {Boolean} true if the options were parsed
   */
  parseConfig(filePath){
    if(fs.existsSync(filePath)){
      if(path.extname(filePath).toLocaleLowerCase() == '.json'){
        let fileContents = fs.readFileSync(filePath);
        let configData;

        try{
          configData = JSON.parse(fileContents);
        }
        catch(err){
          console.log('Error while parsing JSON: ', err);
          process.exit(-1);
        }

        this.input = configData.input;
        this.output = configData.output ? configData.output : Options.DEFAULT_OUTPUT;
        this.lang = configData.lang ? configData.lang : Options.DEFAULT_LANG;
        this.stylesheet = configData.s;

      }
      else {
        console.log("Config file must be JSON!", path.extname(filePath));
        process.exit(-1);
      }
    }
    else {
      console.log(`${filePath} not found! A JSON config file must be supplied`);
      process.exit(-1);
    }
    
    console.log(chalk.green(`Options successfully retrieved from ${filePath}`));
    return true;
  }
}
```

Adding methods to validate the options:

```js
class Options{
  //Returns true if the input path is validated, otherwise throws an error
  validateInput(){
    if(!fs.existsSync(this.input)){
      throw new Error(`${this.input} does not exist. Input path must be a file or directory`);
    }
    return true;
  }

  //Returns true if the output path is validated, otherwise throws an error
  validateOutput(){
    if(this.output != Options.DEFAULT_OUTPUT){
      if(fs.existsSync((this.output))){
        if(!fs.lstatSync((this.output)).isDirectory()){
          throw new Error("Output path points to a file. Output directory must be valid")
        }
      }
      else throw new Error(`${this.output} does not exist. Output directory must be valid`);
    }
    return true;
  }
}
```

After I had written the `Options` module, it was now time to **use it** in `index.js` to parse and process the options supplied to the program. Starting off by **importing** the module:

```js
const { Options } = require("./configOptions");
```
With the help of the `Options` module, I was able to simplify the messy `index.js` by **reducing** the options processing from **46 lines** to **just 12 lines**:
```js
.check((argv) => {
    options = new Options(argv.i, argv.o, argv.s, argv.l);

    if(argv.c){
      //Parse and use options from the config file
      options.parseConfig(argv.c);
    }

    //Validate the options
    return options.validateInput() && options.validateOutput();
  })
  .argv;
```
Since I was now using a variable called `options` to **store** the tool options, all I had to do was to change the **arguments** for `.html` generation:
```js
generateHtml(options.input, options.output, options.stylesheet, options.lang);
```

And **Voila!** `index.js` was looking **clean** again and everything still worked just like before. The final change I made was to move `generateHtml.js` and `configOptions.js` to `bin/modules`

## Organizing changes and rebasing
The **refactoring** work I mentioned above was done in the form of **multiple** separate **commits**. I had a total of **5 commits** by the time I had finished:
```
Fixing the error handling bug introduced by the config feature

Refactor options validation and parsing into a new Options class
    
Move the HTML generation and tool options module to bin
    
Refactor paths for options and generate html modules
    
Improve linting and indentation
```

Before I merged these changes into main, I **squashed** them into a single commit using 
```
git rebase main -i
```
I also ended up **modifying** its commit message to highlight the work done using `git commit --amend`.
At this point, I just performed a **merge** on the `main` branch to **integrate** the changes from my `refactoring` branch and pushed these changes to GitHub.

**Final commit on main for refactoring work:** [b406d2f](https://github.com/dhillonks/cli-ssg/commit/b406d2f2246d1ffeed0eda61e70eba24efa10683)