Level 1 to 3 (Week 2) #1
Conversation
Level 5 created on branch To Update master branch with Level 5 increment.:wq
src/main/java/CommandHandler.java
Outdated
|
|
||
| public class commandHandler { | ||
| DisplayHandler DisplayUnit = new DisplayHandler(); | ||
| InputParser InputUnit = new InputParser(); |
There was a problem hiding this comment.
According to the naming convention, Variable names must be in camelCase.
displayUnit, inputUnit and etc.
There was a problem hiding this comment.
Thanks for pointing this out. I missed this while refactoring.
src/main/java/InputParser.java
Outdated
| @@ -1,4 +1,4 @@ | |||
| public class inputParser { | |||
| public class InputParser { | |||
|
|
|||
| // Check if First Word is Keyword | |||
| public boolean checkIfKeyWord(String Input) { | |||
There was a problem hiding this comment.
if is boolean, hasKeyword/isKeyword would be a better naming?
InputParser.isKeyword("abc").
There was a problem hiding this comment.
Yes. Indeed it would be. Thank you.
src/main/java/duke/Duke.java
Outdated
| displayHandler.DisplayInvalidInput(); | ||
| // If input is a recognised Keyword | ||
| else | ||
| command.checkCommandType(keyWord, body, list); |
There was a problem hiding this comment.
Is it good to have curly brackets for if-else statement
|
|
||
| return task; | ||
|
|
||
| } catch (NumberFormatException e) { } catch (ArrayIndexOutOfBoundsException e) { } |
There was a problem hiding this comment.
Is it good to have error message in the bracket?
There was a problem hiding this comment.
Is it good to rearrange the second catch below the first catch statement?
| import duke.input.InputHandler; | ||
| import duke.input.InputParser; | ||
| import duke.output.DisplayHandler; | ||
| import duke.storage.ListHandler; |
There was a problem hiding this comment.
Perhaps the ordering of the import statements could be reworked to align with the instantiation and dependencies below just before the while loop and inside of it as well ?
| @@ -0,0 +1,11 @@ | |||
| package duke.command; | |||
|
|
|||
| public enum KEYWORD { | |||
There was a problem hiding this comment.
Perhaps we can use the PascalCase here for enum ?
Reference: https://se-education.org/guides/conventions/java/index.html#naming
To Merge Branch Level 6 with Master
Merging branch-Level-7 with master .
branch-A-JUnit - (add Junit testcases)
Individual project. Duke. Pull Request. Week 2 Submission.
Based upon: https://nus-tic4001-ay2021s1.github.io/website/schedule/week3/project.html