Skip to content

Code review#14

Open
JamesB05 wants to merge 1 commit into
cfrantzidis:masterfrom
JamesB05:master
Open

Code review#14
JamesB05 wants to merge 1 commit into
cfrantzidis:masterfrom
JamesB05:master

Conversation

@JamesB05

Copy link
Copy Markdown
  1. There are some comments in this code, however I think adding a summary and more detailed comments are needed to improve readability.

  2. The check to see if the file exists is done well and will handle any related errors accordingly. On the other hand, there are no other error checks, I would suggest adding a check to see if the user has made a valid input, as without one this could cause the code to crash if the user leaves the string empty.

  3. The code is straightforward and simplistic. For improved modularity, i think you would benefit from using more OOP structures like classes as having everything in Main() can cause the code to become hard to reuse and test in the future. Separating the code also allows new features like saving results to be added much easier.

  4. Variable names are descriptive and follow camelCase conventions, this greatly helps with the codes readability.

  5. the code does check to see if the file exists, however it does not account for if the file is readable or formatted properly. Using StringSplitOptions.RemoveEmotyEntries would improve the way the code splits while also avoiding empty strings in the word arrays.

  6. The code outputs in alphabetical order as intended, it also shows total unique words clearly. However it is very simplistic. All I would suggest is some formatting for readability.

  7. The cide uses Regex correctly yo remove punctuation. It also converts to lower case allowing for case-insensitive counting. The code uses a dictionary appropriately to count the words in the file, and the splitting is good for typical text files. Overall the program logic is well integrated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant