DEV Community

Oliver Pham
Oliver Pham

Posted on

How I Refactored my Code

This week, I noticed that some functions in my static site generator (SSG) were hardcoded with complex logic and "magic values", so I decided to focus on refactoring them. Without cleaning them up, maintaining them would be a tragedy. For instance, there was a function spanning 36 lines of code with 8 if/elif statements. Some of the statements even have nested if/elif statements themselves. You can find the function referenced in this issue.

Code Refactoring

To resolve the above issue, I thought the best approach was to avoid reinventing the wheel and save myself hours of debugging: use a third-party library. After implementing a Python implementation of John Gruber’s Markdown, 36 lines of code were cut down to a single function call. I've not benchmarked my SSG after the change, but in terms of code readability, it's certainly worth the overhead caused by the library.

Another part that needed to be refactored was the function for extracting a file name without its (multiple) extensions from a file path. For some reason, there was confusing and irrelevant validation of the file extension inside the function.

# Inside the function
if pathlib.Path(file_path.split('.')[0]).stem == ".md" or pathlib.Path(file_path.split('.')[0]).stem == ".txt":
  return pathlib.Path(file_path.split('.')[0]).stem 
elif pathlib.Path(file_path.split('.')[1]).stem == ".md" or pathlib.Path(file_path.split('.')[1]).stem == ".md": 
  return pathlib.Path(file_path.split('.')[0]).stem 
elif pathlib.Path(file_path.split('.')[1]).stem == ".json" or pathlib.Path(file_path.split('.')[1]).stem == ".ini": 
  return pathlib.Path(file_path.split('.')[0]).stem 
else: 
  return pathlib.Path(file_path.split('.')[0]).stem
Enter fullscreen mode Exit fullscreen mode

Since that's already checked at the beginning of the program, I simply removed the redundant checks.

The last piece of refactoring was probably the most problematic one. There was a block of code for setting the value of the options read from a JSON configuration file for generating an HTML document.

 def silkie(input_path, stylesheet, lang, config): 
     """Static site generator with the smoothness of silk""" 
     try: 
         # Set options for generating HTML document from config file
         if config is not None: 
             with open(config, "r", encoding="utf-8") as f: 
                 config_item = json.load(f) 
                 if "input" in config_item: 
                     input_path = config_item["input"] 
                 else: 
                     input_path = None 
                 if "stylesheet" in config_item: 
                     stylesheet = config_item["stylesheet"] 
                 if "lang" in config_item: 
                     lang = config_item["lang"] 

         kwargs = dict(input_path=input_path, 
                       stylesheet_url=stylesheet, lang=lang) 
         options = GeneratorOptions( 
             **{k: v for k, v in kwargs.items() if v is not None}) 
         # Clean build 
         shutil.rmtree(DIST_DIRECTORY_PATH, ignore_errors=True) 
         makedirs(DIST_DIRECTORY_PATH, exist_ok=True) 
         # Generate static file(s) 
         if path.isfile(input_path) and is_filetype_supported(input_path): 
             generate_static_file(options) 
         if path.isdir(input_path): 
             for extension in SUPORTED_FILE_EXTENSIONS: 
                 for filepath in glob.glob(path.join(input_path, "*" + extension)): 
                     options.input_path = filepath 
                     generate_static_file(options) 
     except OSError as e: 
         click.echo( 
             f"{TextColor.FAIL}\u2715 Error: Build directory can't be created!{TextColor.ENDC}") 
     except FileNotFoundError: 
         print(f'Error: There is no Json File "{config}" !') 
     except json.JSONDecodeError: 
         print(f'Error:Invalid JSON syntax!" ') 
Enter fullscreen mode Exit fullscreen mode

I found this too irrelevant to be included in the main method, so I extracted to a method inside the GeneratorOptions class.

During the process of refactoring, I noticed a logic flaw with the above code. If the input_path was not included in the configuration file, it could crash the program (due to input_path being set to None). Thankfully, it was fixed before my code was committed. With that done, it was time for rebasing and squashing!

Git Interactive Rebase

It is indeed unnecessary to keep all the commits of each iteration of refactoring, so squashing them sounds like a good idea. After entering git rebase main -i into my command line, I edited my commit message one last time to make sure it's detailed and clear enough. After squashing all my commits into a single commit, I could finally merge my refactoring branch, where I'd been refactoring my code, into the main branch.

Conclusion

Luckily, throughout the code refactoring process, I didn't encounter any other major issues or crashes. It was not only a good chance to clean up my project but also a wonderful opportunity for me to learn more about Git Interactive Rebase and amending a commit.

Top comments (0)