[antrikshdhand] iP#72
Conversation
| public int processLine(String line) { | ||
| if (line.equalsIgnoreCase("bye")) { | ||
| sayGoodbye(); | ||
| return 1; |
There was a problem hiding this comment.
Should this be changed to a boolean instead of a number to avoid magic number?
| System.out.println("___________________________________________________________________________"); | ||
| } | ||
|
|
||
| return 0; |
There was a problem hiding this comment.
Should this be changed to return 'false' instead of 0 to avoid magic number?
| while (scan.hasNextLine()) { | ||
| line = scan.nextLine(); | ||
|
|
||
| if (herbert.processLine(line) == 1) return; |
There was a problem hiding this comment.
Should this be separated into two lines to follow the coding standards?
There was a problem hiding this comment.
Yes, you're right. I'm not sure how I missed this 😅
| System.out.println("___________________________________________________________________________"); | ||
| } | ||
|
|
||
| public void sayGoodbye() { |
There was a problem hiding this comment.
Good job. I like how you named this method, which follows the coding standard.
|
Thank you for reviewing my PR. I will update my code with your recommendations! |
maanyos
left a comment
maanyos
left a comment
There was a problem hiding this comment.
Overall, good adherence to method & variable naming standard, with a few exceptions.
Code can be improved by applying OOP principles, such as inheritance and exceptions as taught in (future) lectures to improve clarity.
Lastly, avoid big classes that do a lot, try to split the components by their use.
| @@ -0,0 +1,291 @@ | |||
| package duke; | |||
|
|
|||
| import task.*; | |||
| + "|__| |__||_______||___| |_||_______||_______||___| |_| |___| "; | ||
|
|
||
| System.out.println(System.lineSeparator() + logo); | ||
| System.out.println("___________________________________________________________________________"); |
There was a problem hiding this comment.
consider refactoring this? seems to be used frequently
| System.out.println("___________________________________________________________________________"); | ||
| System.out.println("\tHello! I'm Herbert." + System.lineSeparator() + "\tWhat can I do for you?"); | ||
| System.out.println(System.lineSeparator() + "\tYou may choose from the following commands:"); | ||
| for (Command s : Command.values()) { |
There was a problem hiding this comment.
try to use more meaningful variable names. can make code a lot clearer if the loop body is long
| System.out.println("###########################################################################"); | ||
| } | ||
|
|
||
| public int processLine(String line) { |
There was a problem hiding this comment.
returning int from a processLine method isn't very informative
| @@ -0,0 +1,68 @@ | |||
| package duke; | |||
|
|
|||
| public enum Command { | |||
| return 0; | ||
| } | ||
|
|
||
| private void markTask(String line, boolean completed) { |
| int process = herbert.processLine(line); | ||
| if (process == 1) { |
There was a problem hiding this comment.
clarity can be improved by using another return type as mentioned above
| } | ||
|
|
||
| public String getStatusIcon() { | ||
| return (completed ? "X" : " "); |
There was a problem hiding this comment.
elaborating this line can improve readibility
Add proper date functionality to Deadline and Event classes
3 participants
No description provided.