DPS909 Blog - Lab 1: Code Review
Alexander Samaniego
Posted on September 16, 2022
This week for my Open Source course (DPS909), I completed my first lab of the semester. The purpose of the first lab was to get us to start working together on a project as a community.
Background
The course contains a major project that we are supposed to work on for the length of the term. The first project deliverable, release 0.1, requires us to build a simple Static Site Generator (SSG) as a command-line tool.
For the majority of the time, release 0.1 was an individual project. Lab 1 is meant for us to start collaborating with others in the community by thoroughly testing and reviewing other people's release 0.1 code.
Review Process and Outcomes
Reviewing and Testing Someone's Project
In order to find other's to collaborate with, I used the DPS909 Slack workspace. I ended up collaborating with rudychung who made their SSG using C++.
Testing and reviewing another person's code was a lot more challenging than I initially thought. At the beginning it was difficult because the SSG tool I was reviewing was built using a different tech stack than mine. While I am familiar with C++, it still took a bit of time to figure out exactly how their program worked.
After getting over the initial hurdle, the next challenge was thinking of ways to break the program. It was difficult because I know how the program should work. So I had to put myself in the shoes of somebody who had no idea what they were doing. I did this by following the README.MD instructions step-by-step and seeing if something goes wrong.
The thing that really surprised me was just how difficult it was to find issues in the code. Mainly because I had the initial mindset that code issues had to be program breaking issues. Therefore, I felt like any small issue was just 'nitpicking' and not worthy of being an issue.
The issues and bugs that I did find were minor.
They included:
Issue #1: Multiple new lines add empty unclosed 'p' tags.
In a text file with more than one new line separating paragraphs, unclosed 'p' tags are added. This creates an invalid HTML file. If a text file has more than 1 line separating a paragraph, the file will have 4 end 'p' tags like so:
<h1>Silver Blaze</h1>
<p>I am afraid, Watson, that I shall have to go,” said Holmes, as we sat down together to our breakfast one morning.</p>
<p>“Go! Where to?”234423434</p>
</p>
</p>
</p>
</p>
<p>“To Dartmoor; to King’s Pyland.”</p>
I used W3C Markup Validator to check if the generated HTML file is a valid one.
Issue #2: In README.MD, specify the directory the user needs to move to in order to execute.
This was a simple issue meant to make the instructions more clear to new users. Initially, a user might attempt to input commands on the command-line from any directory. They might not know that they have to move to the directory where the executable (.exe) file is stored to run the program. So this issue is intended to prevent that from happening.
Issue #3: Code clean up: remove redundant condition in main.cpp (line 63).
There was a condition in one of the If statements that appeared to be redundant since the second condition seemed to be enough. However, upon further experimentation, this proved false.
Issue #4: Suggestion: Reduce repetitive code by creating functions for adding HTML header and closing tags.
Another simple issue meant to clean up the code and reduce redundancy. For HTML file generation, the same lines of code used to create the header and closing tags were used twice in two files. I suggested to find a way to reuse the code and prevent repetitiveness.
Issue #5: A blank HTML file is created when inputting a filename that doesn't exist.
This was one of the more serious issues I found. When inputting a file that doesn't exist, a HTML file with nothing in the body will still be created and placed in the appropriate output folder. This is a bug in the program as the tool should not process any file that doesn't exist. No file should be created and no output folder should be deleted or created. I suggested to add a condition that checks whether the input file actually exists before executing the rest of the program.
Getting My Project Reviewed and Tested
Having the project that I created be reviewed by someone else felt a bit nerve-racking at first. It felt as if I would be exposed as a terrible programmer having someone that I didn't know personally look at my project. I thought that they would end up finding many program-breaking bugs that I missed. However, I was surprised that the issues that were found were relatively minor.
Issues found in my project repo included:
Issue #1: Instruction Step Missing
My initial README.MD instructions were not too clear as I had missed a step. Users need to run the command
npm install -g
as the 3rd step in the process to install npm packages.
How I fixed it
I added the crucial step into README.MD to make it more clear to new users.
Issue 2: -v Option Not Working
This issue was a problem regarding two options in my options list. The --version option functioned as intended, but -v did not work. -v is listed in the list of options in README.MD, but does not exist in the help output. The same issue existed for the -h/--help option.
How I fixed it
I added -v and -h as aliases for both the help and version options in index.js:
const options = yargs
.usage("Usage: -i <input>")
.help("help")
.alias("help", "h")
.version(
chalk.bold(`\nName: ${name}\nVersion: ${version}\nAuthor: ${author}\n`)
)
.alias("version", "v")
Issue #3: -v/--version missing tool's name
According to the instructions when using the -v/--version option, it should output the tool's name as well as the version number.
How I fixed it
I added the tool's name, version, and author name to the output by importing variables from package.json.
Issue #4: HTMLcreate unused parameter
This was a minor issue as I had an unused parameter for one of my functions.
How I fixed it
I used the parameter to give the HTML file the same name as the incoming text file.
Issue #5: Blank file name allowed for input
This issue described a bug that occurred when inputting no argument in the command. Running the command without any file name argument will create the 'dist' directory without any files in it. Ideally, no file or folder should be created and an error message should appear.
How I fixed it
I added an error message for when there is no file or directory path as an argument. If it detects this, the program will show the message and exit.
What I Learned
I learned that coding is never "finished". No matter how good or neat someone's program is, there's always room for improvement.
Posted on September 16, 2022
Join Our Newsletter. No Spam, Only the good stuff.
Sign up to receive the latest update from our blog.
Related
November 29, 2024