code review (resit)#11
Open
420sam wants to merge 1 commit into
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Naming:
Variables are more descriptive in the code.
Changes to make - While the naming is clear it could be made better. One-word variables e.g. lines could be renamed as file lines.
Code structure:
While all being in main the program is split based on the separate's functions and logic. The developer can easily identify which part of the code is responsible for what.
Changes to make - Instead of it all being in one main method it would be easier to test etc. If the code was split into more methods. The separate methods could be for some of the main identifiable functions such as a reading file lines.
Error handling:
With some error handling (checking for existing files) there is a reduction in overall crashes of the program at every stage. There is also feedbacked to let the user know what exactly the issue is what when they when they tried to do something in the program.
Changes to make - There is improvement for security errors for user authorizations/ identification. And some of the user feedback could be clearer and to the point.
File assumption:
There is basic file assumption, the white spaces are assumed to be splitting and visible to the user. Punctuation removes is also more robust as it is implemented through regex which works on standard text.
Changes to make - The file assumption has some limitations regardless such as the user not putting in Aski but uni code such as emojis.
OOP Principles:
The code is not object oriented but procedural all the code is in main with no other methods.
Changes to make - The code can be more separated instead of it all being in main, there being more classes or methods would be better.
Output:
The program remains to still be user friendly as it whole it outputs error explanation it also outputs helpful prompts to the user on that to do to further continuing in the program. Also the overall result output is cleaner being in alphabetical order and meeting all the output requirements.
Changes to make - Output showing total wordcount and there being more output options would be an improvement.
Program logic:
All requirements are met the program removes punctuation, converts everything to lowercase, splits on whitespace and correctly counts word occurrences.
Changes to make - while the program works correctly improvements can be made by improving word splitting (including across lines) and normalizing words further than just lower cases.