[Ong JunZheng] iP#114
Conversation
LuxLucky7
left a comment
LuxLucky7
left a comment
There was a problem hiding this comment.
LGTM overall! Do check the indendation nits
| @@ -0,0 +1,34 @@ | |||
| public class Task { | |||
There was a problem hiding this comment.
I like how clean this class is written!
| task.setNotDone(); | ||
|
|
||
| String message = "\t" + "OK, I've marked this task as not done yet:" | ||
| + System.lineSeparator() + "\t\t" + task; |
There was a problem hiding this comment.
I like that you've used the lineSeperator() method!
| Task task; | ||
|
|
||
| switch (commandLineParts[0].toLowerCase()) { | ||
| case "bye": |
There was a problem hiding this comment.
Please do check if the indendation for the case after the switch is correct
| @@ -19,15 +19,29 @@ public void run() { | |||
| System.out.print("Enter command: \t"); | |||
There was a problem hiding this comment.
I like the use of the empty lines within methods to differentiate between different tasks!
| commandLine = sc.nextLine().trim(); | ||
|
|
||
| switch (commandLine.toLowerCase()) { | ||
| String[] commandLineParts = commandLine.split(" ", 2); |
There was a problem hiding this comment.
can this number 2 be put as a constant?
| } else { | ||
| return "[ ]"; | ||
| } | ||
| } |
| break; | ||
| default: | ||
| addTask(commandLine); | ||
| break; |
There was a problem hiding this comment.
nice change to make this more readable!
| + System.lineSeparator() + "\t\t" + task; | ||
| echo(message); | ||
| } | ||
|
|
| package quinn.task; | ||
|
|
||
| public class Deadline extends Task { | ||
| private final String byDateTimeInput; |
There was a problem hiding this comment.
Perhaps using dueDateTime is more descriptive and concise here. Furthermore adding the word 'input' in the naming dosent't add significant value and can make the code more verbose.
| private final String fromDateTimeInput; | ||
| private final String toDateTimeInput; |
There was a problem hiding this comment.
Perhaps using startDateTime and endDateTime is more descriptive and concise here. Furthermore adding the word 'input' in the naming dosent't add significant value and can make the code more verbose.
| } | ||
| } | ||
|
|
||
| public void processCommand(String commandLine) throws QuinnException, IOException { |
There was a problem hiding this comment.
Perhaps it is better to use a more specific name for this method such as checkAndSplitCommand as process might not be a very specific term. See https://nus-cs2113-ay2425s1.github.io/website/se-book-adapted/chapters/codeQuality.html#guideline-name-well
Implement Level-8 for chatbot to understand date time
Implement Level-9
Added JavaDoc comments
4 participants
No description provided.