join implementation

[DemetriusChatterjee]

The documentation can be quickly accessed from the README on the repo page or just by navigating to the docs directory and then opening documentation.md.

[aryianj]

Your join works well! However, your documentation should be clearer on what join does, how it works, and the expected output of the tests. Although extensive print statements are good to let the user know what's going on, they can feel overwhelming. Additionally, for lines 291 - 302 or 306 - 318, you could get by with one if statement where count1/count2 <= 0 and print the usage guidelines. There are also some indentation issues within your code. Also, a --help command could be useful for your code. Overall, very great job!

[DemetriusChatterjee]

Your join works well! However, your documentation should be clearer on what join does, how it works, and the expected output of the tests. Although extensive print statements are good to let the user know what's going on, they can feel overwhelming. Additionally, for lines 291 - 302 or 306 - 318, you could get by with one if statement where count1/count2 <= 0 and print the usage guidelines. There are also some indentation issues within your code. Also, a --help command could be useful for your code. Overall, very great job!

Addressed changes in documentation, join (adding help flag and addressed minor indentation issues and made the user feedback when saving results to a file less overwhelming by reducing the amount of print statements.)

[RebeccaLLittle]

Verification
This project is very robustly done! Great work overall. It seems as though this project meets all the expectations set from the issue.

Testing
It may be a me issue, but the testing documentation was a little hard to read and I was hoping for maybe a full small test example. For example, all of file1, file2 and what it made after join. I was just scrolling back and forth between the files themselvies and the documentation to figure it out.

Also, they results I got when running the documentation tests seemed to slightly vary from the expected output

Documentation expects join file1.txt file2.txt to produce 25 joined lines; actual output shows “No matches found”.
Documentation expects join file1.txt file2.txt output.txt to save 25 lines and display them; actual output shows empty results.
I could also be misinterpreting the documentation or missing a step, but these were the results I got.

Code Walkthrough
The program overall looks pretty good! I think main() could be a little cleaner, as there is a lot of duplicate error handling code (lines 312-355). Well done!

Formatting
Format is great nice work! I appreciate all the informative comments

Checklist
High Level Checks:

• [ ❌ ] PR fully resolves the task/issue
• [ ✅] Does the PR have tests?
• [ ✅ ] Does the PR follow project code formatting standards?
• [ ✅ ] Does the OS still compile and run?
• [ ✅] Is documentation included?

Code Checks:

• [ ✅] Does the code achieve its intended purpose?
• [✅ ] Were all edge cases considered?
• [ ✅ ] Is the code organized and maintainable?
• [ ✅ ] Does the code maintain the same level of performance?

[malensek]

Looks good, and thanks for addressing some of the issues already! This is a cool idea for a project.

My main misgivings are: the readme is... explaining how to build xv6 etc, talking about toolchains etc. It should just cover join.

The other issue is your fixed-sized buffers are pretty small, and that's going to limit the usefulness of this. I've had a few projects add dynamic allocation but in your case this is a complicated enough project to not warrant that. However, there's no reason not to use constants / #define so that your code is not peppered with random 50, 256, 255 ints all over the place. That way if this class someday upgrades to an even fancier old unix OS (maybe it's time for xv7) we can update the buffer sizes and enjoy extended joining... :-)

4.9/5

+0.1 -> create some constants for your buffer sizes, clean up the docs and help message, and do a final sweep to clean up unnecessary comments.