My First Code Review

Amir Mullagaliev - Sep 14 - - Dev Community

First async code review

I've never been given a chance to be the one who reviews the code, as a student I always receive a feedback from professors about my code. This time I've an opportunity to switch the role and review the code first time. Forked the project, cloned it, and following all instructions executed the code. I'd been frozen for first 15 minutes, then started exploring the code line by line, what helped me to understand the logic. I felt that I am capable of doing this because I didn't have a time frame, so I realized that doing it asynchronously is a delightful thing, nobody is waiting for you, it's just you and the code.

Testing

Once I understood the logic, proceed to the testing. I started feeling nervous, for the reason, that code was well-written, and I couldn't find any bugs or issues. Thoughts of that I wasn't able to find issues were rushing my head. However, I reread the documentation of "Lab1". There our professor mentioned: "There is no perfect code, everything can be improved." It gave me a lot of confidence, so I decided to look up at other students' repositories, which I considered unmatchable(perfect). I checked them out and realized, that other people were able to find and post a lot of issues. It really surprised me. After that I found just 3 issues, will talk about them later.

My code was reviewed

After finding a partner for this lab, we shared our repositories to each other. Couple of hours later, I've received 4 emails in a row, they were hitting my head as a hammer, my partner started posting the issues inside of my repo. I wasn't ready didn't know how to react, I thought: "Is he blaming my project, or trying to help?!" After some time, I refreshed my mind and started thinking logically, he, definitely, was trying to help, but I still was surprised that I had something wrong.

I've received 4 issues

Issue #1 my partner mentioned that my README.MD didn't have an instruction how to use the application. I completely forgot that someone is going to use my tool, nobody ever used my projects, moreover, nobody ever seen them, beside my professors, which they never ran on their machines. It lead me to not creating appropriate documentation. I have never had experience using markdown format, it was a trouble. However, using magic of internet I found one of the guides, and following it created my first README.MD file.

Issue #2 it was a good point by my partner, I didn't have a functionality of using flags like -v(--version) and -h(--help). However, It has been already implemented, waiting for my push.

Issue #3 this issue was regarding issue 1, I didn't provide a documentation on how to make the polyglot command globally accessible. It wasn't hard to fix, just added to my README.MD steps on how to start the application.

Issue #4 first bug that was found by my partner, I knew about it, but was working on other features because I knew it was easy to fix. Let me break you down the issue, whenever you pass invalid file that doesn't even exist to the application using cli, for example:

 ./polyglot invalidFile.txt java
Enter fullscreen mode Exit fullscreen mode

My application wouldn't stop and throw an exception, it was generating some weird output, mentioning "File doesn't exist" :

Image description

As I told you, easy fix, my last commit was aiming on getting rid of all issues found by my partner. Thank you, Henrique!

Issues that I found

As mentioned above, I found only 3 issues. After reviewing and testing the application, I felt that it was my ceiling, just for now, and I gave up :c

Issue #1 as one of our requirements for this release is that we must provide to the users the folder called examples where we store example files to preview and test the usage of tool. My partner was missing it, so I wrote a detailed suggestion.

Issue #2, the response goes exclusively to the file. However, as one of the requirements for this project, that the results should be also displayed in the terminal(cli), as it's implementation of command-line tool. I suggested him to just add one line of code:

print(f"Result is: {readme_content}")
Enter fullscreen mode Exit fullscreen mode

Issue #3 currently, the program prints all errors and exceptions to stdout. Best practice recommend sending error messages to stderr, allowing proper separation of stdout from error output. My suggestion is:

  1. Import sys Module:
    Add import sys to access the stderr stream

  2. Redirect exception messages to stderr:

Replace:

printf(f"An unexpected error occured: {e}")
Enter fullscreen mode Exit fullscreen mode

With

printf(f"An unexpected error occured: {e}", file=sys.stderr)
Enter fullscreen mode Exit fullscreen mode

Conclusion

Overall, I really liked the process of reviewing classmate's application and getting feedback for my own. It gave me a good experience of how to review the code in the future. I was able to fix all my issues. I learnt how to create Issues and review someone's code. It was fun! Hope I will get some more of this experience!

. . . .
Terabox Video Player