Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Minor changes to Code Quality #295

Merged
merged 13 commits into from
Apr 15, 2024

Conversation

nur-haziq
Copy link

@nur-haziq nur-haziq commented Apr 15, 2024

Summary of Changes:

Storage

  • Catch ArrayIndexOutOfBoundsException. Thanks @YHWong20 for the catch.

RestockCommand and SellCommand

  • Remove unused RegEx pattern, and unused imports

ItemList

Parser

  • Included explicit //Fallthrough statement for the 2 initial switch cases that do not have a break statement

UpdateCommandParser

  • Rename methods to sound more 'verb-like'

General

  • Added and removed blank lines where necessary



For the catching of exceptions in Storage

There is an alternative solution, which is deemed as bad programming practice in most cases, which is to catch the general exception catch (Exception e).

However, in scenarios like file parsing where input can be quite variable and potentially out of the control of our application (e.g., user-modified files), it could be prudent to include such handling to ensure the application remains robust against all forms of bad input. Ultimately, all the exceptions during loading will be handled in the same way: setting isCorrupted to True, and creating a new items.txt file.

Do LMK you guys' opinion on this.

Copy link

@YHWong20 YHWong20 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I reckon that it's better to do it this way (as opposed to catching Exception e - just for the sake of keeping our code quality relatively high). Hopefully this is the last exception that we'll need to deal with haha

@nur-haziq nur-haziq changed the title Catch ArrayIndexOutOfBoundsException when loading up data from storage Minor changes to Code Quality Apr 15, 2024
@nur-haziq nur-haziq added this to the v2.1 milestone Apr 15, 2024
Comment on lines +63 to +65
//Fallthrough
case "exit":
//Fallthrough

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch!

Copy link

@YHWong20 YHWong20 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for making the necessary changes to improve our code quality!

@PureUsagi PureUsagi merged commit d5d4d26 into AY2324S2-CS2113T-T09-2:master Apr 15, 2024
3 checks passed
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.

minor code quality issues
3 participants