Code Review

So this week my lab in my opensource development course required to get my cli app to get reviewed and to review another student's repository. The first part of finding a partner was easy as one of my good friends Inderpreet Singh Parmar is taking this course (dps 909 at Seneca) as well. I'm making a cli app that takes text files and either summarize, format or paraphrase the document and produces a new document. Inder's Tailor4Job is done in python and the base concept is that it takes in resume and specially customizes it for a specific job posting. We hopped on a call and explained our code to each other and then we decided to do code review asynchronously.

Inder's Repo

Inder’s app, like mine, is unfinished; however, I did find some issues that I thought Inder would find useful. It was relatively easy to fork and clone Inder’s repo and run the code. Although I would’ve preferred a requirements file, Inder said he didn’t have the time to work on it.

1. No validation for input files
   I found this to be a crucial issue since Inder should have some validation unless it is highly likely the app would crash.

2. Hardcoding File Format Constraints
   This is a suggestion more than a issue. I think Inder intended the app to only take docx and pdf input however I believe hardcoding is never the way to go. This decreases the modularity and maintainability (It would also be a pain if I had to contribute to this repo)

3. Error Handling in File Reading
   This I believe to be a flaw of the program though I expect Inder to already know about this. The current implementation does not handle errors related to file access, such as if the file doesn't exist, if it's locked, or if it's unreadable. This should be addressed as the whole point of the app then becomes moot.

My Cli App

Before Inder looked at my cli app I informed him that the app is not complete. Only the basic menu option has been done. So the code is pretty much basic C++ programming. I didn't think highly of my code since I didn't spend too much time on it (Roughly 2 hours) and my C++ was rusty but I wanted to do it in C++ since I have been programming in Laravel all summer for my internship so I was just tired doing web development. Not too my surprise there were quite some issues Inder informed me about. They are listed below

1. API key as a class member without security handling:
   So this was a big one. To be honest when writing this code I didn't give much thought about security and now I realize this mistake was a big one! I would've lost my job if I was working somewhere. I'm grateful to Inder to point this out. This would be a reminder for me to not post personal data especially when doing open-source work.

2. Use of conio.h:
   I don't agree with this issue. I used conio.h because it allowed me to design the cli how I wanted it to be. Inder suggested to remove conio.h and use modern alternatives. If I define how to use conio.h with other platforms (like linux) it shouldn't be a problem right?

3. Cannot input multiple files
   I absolutely owe Inder for this issue since it would've cost me some marks for my release 0.1 of this app. I guess this is what code reviews are for!!!

End remarks

I did close all my issues however, issue #1 and #3 still needs work. This has not been my first my code review but this was the most asynchronous one that I did. It was refreshing to see Inder's code and help out for once instead of asking for it. This process helped me realize the importance of collaboration and how you have to look at the bigger picture when thinking about open source.