Refactoring Code

Hey Everyone! If you've been keeping up then you know of Rssg my noob level static site generator for my class OSD600 :) The previous post can be found here

Adding New Features to Rssg and Git Merging Branches

Antonio-Bennett ・ Oct 1 '21 ・ 2 min read

#rust #opensource #github #osd600

Task

This week we are supposed to find 3 ways to refractor our code such as renaming variables, separating logic and etc. The goal in doing so is not only to improve our code but also to practice rebasing a git branch. For rssg I decided on refractoring by

• returning process exit 0 for main program if there was an error
• renaming read_text function to be more specfic on what it does
• separated the logic in the process function so it was more readable

Steps

As always if you just want to view the commit is is here

#1

My very first step was refractoring this bad boy

pub fn run() {
    //Takes appropriate action based on arguments passed ex. rssg --input
    read_arguments().unwrap();
}

read_arguments() has code that matches the cli argument and returns an Err if needed however I was calling program::exit(0) in there such as

if env::args().count() == 2 {
    println!("Please enter input. Type rssg --help or -h for more information.");
    process::exit(0);
}

when I could just return the Err like needed

return Err("Please enter input. Type rssg --help or -h for more information.".into());

and then call read_arguments like this

pub fn run() {
    //Takes appropriate action based on arguments passed ex. rssg --input
    if read_arguments().is_err() {
        process::exit(0);
    }
}

#2

This one was very simple but it made a lot of sense

I have a function named read_text that goes through each argument and calls the process function if the argument is a valid file.

pub fn read_text(args: &[String]) -> Result<(), Box<dyn Error>> {
    //Iterate through each input and process
    args.iter().for_each(|arg| {
        if let Ok(mut file) = fs::read_to_string(arg.to_owned()) {
            //Reaches this if the argument was just a filename
            process(&mut file, arg);
        } else if Path::new(arg).is_dir() {
            //Argument is a directory so we have to recursively search the dir
            let path = Path::new(arg);
            visit_dirs(path, &process).expect("Couldn't convert dir");
        }
    });

    Ok(())
}

It is much more appropriately named as process_arguments

#3

Previously my process function which turns the input file into an html file was suuuuuuper convoluted because it basically had an if check with logic in both the if and else for files with and without titles. I separated this logic into 2 separate functions and call them inside the if check instead. This makes the code more readable and easy to follow.

    if vec_lines[1].is_empty() && vec_lines[2].is_empty() && !vec_lines[0].is_empty() {
        process_file_with_title(
            vec_lines,
            &mut html,
            &mut firstline,
            &mut is_header,
            &mut line,
            prev_tag,
            ext,
        );
    } else {
        process_file_with_no_title(
            &name,
            default_content,
            vec_lines,
            &mut html,
            &mut firstline,
            &mut is_header,
            &mut line,
            prev_tag,
            ext,
        )
    }

Rebasing

This was a pretty cool process. You can rebase to pretty much combine multiple commits into one by doing git rebase main/master -i while on the checked out branch. It is a very cool git command to know but I'm not sure on how often I'd honestly use it. It is great though to have it as a tool that I could possibly use in the future :)