[Lester Sim Jia Jun] iP - #77
lestersimjj wants to merge 90 commits into
Conversation
| @@ -0,0 +1,67 @@ | |||
| import java.util.ArrayList; | |||
|
|
|||
| public class ChatSession { | |||
There was a problem hiding this comment.
Good use of ChatSession class to abstract commands
| } | ||
|
|
||
| // Execute user commands | ||
| switch(userCommand) { |
There was a problem hiding this comment.
Good catch on the switch-case indentation standard!
| } | ||
|
|
||
| public void markTaskIndex(int taskID) { | ||
| String output = taskList.get(taskID - 1).markTask(); // zero-based indexing |
| } | ||
|
|
||
| public void unmarkTaskIndex(int taskID) { | ||
| String output = taskList.get(taskID - 1).unmarkTask(); // zero-based indexing |
There was a problem hiding this comment.
Once again can move your comments above the line
| @@ -0,0 +1,67 @@ | |||
| import java.util.ArrayList; | |||
There was a problem hiding this comment.
Good job on only importing the specific class needed!
| userInputArr = userInput.split(" ", 2); | ||
| userCommand = userInputArr[0]; | ||
|
|
||
| if (userCommand.startsWith("bye")) { |
There was a problem hiding this comment.
all methods, conditionals, and loops seem to follow the K&R style brackets, and statements within are all in the next line so that's great
| // Create a new Task, append to taskList | ||
| // Task newTask = new Task(description); |
There was a problem hiding this comment.
Maybe the indentation of these comments could be in line with the code! It seems a little messy here but small issue
|
|
||
| public class ChatSession { | ||
| // Store subclasses of tasks | ||
| ArrayList<Task> taskList; |
Error Handling For: Missing arguments in todo Unrecognised commands
|
|
||
| switch (taskType) { | ||
| case "T": | ||
| currChat.addInitialTask(new Todo(isDone, userInputArr[2])); | ||
| break; | ||
| case "D": | ||
| currChat.addInitialTask(new Deadline(isDone, userInputArr[2], userInputArr[3])); | ||
| break; | ||
| case "E": | ||
| currChat.addInitialTask(new Event(isDone, userInputArr[2], userInputArr[3])); | ||
| break; | ||
| } | ||
| } |
There was a problem hiding this comment.
Include the default branch in case statements.
| switch (userCommand) { | ||
| case "list": | ||
| currChat.printTaskList(); | ||
| break; | ||
| case "mark": | ||
| // Additional argument provided by user is the task no. to mark | ||
| currChat.markTaskIndex(Integer.parseInt(userInputArr[1])); |
There was a problem hiding this comment.
Consider extracting magic strings such as "list", "mark" into constants.
| // Create a new Task, append to taskList | ||
| taskList.add(task); |
There was a problem hiding this comment.
Avoid unnecessary comments where the code is self-explanatory.
| break; | ||
| case "deadline": | ||
| // eg. return book /by Sunday | ||
| userArguments = userInputArr[1].split(" /by ", 2); | ||
| description = userArguments[0]; //eg. return book | ||
| String by = userArguments[1]; // eg. Sunday | ||
| currChat.addTask(new Deadline(description, by)); | ||
| break; | ||
| case "event": | ||
| // eg. project meeting /at Mon 2-4pm | ||
| userArguments = userInputArr[1].split(" /at ", 2); | ||
| description = userArguments[0]; // eg. project meeting | ||
| String eventTime = userArguments[1]; // eg. Mon 2-4pm | ||
| currChat.addTask(new Event(description, eventTime)); | ||
| break; | ||
| case "delete": | ||
| currChat.deleteTaskIndex(Integer.parseInt(userInputArr[1])); | ||
| break; |
There was a problem hiding this comment.
Should there be input validation for the userArguments?
| } | ||
|
|
||
| // Return the added task | ||
| System.out.println("____________________________________________________________"); |
There was a problem hiding this comment.
Consider extracting repeated strings like this into constants for better reusability.
…eption if exceed list size.
…ns a string for UI to output
4 participants
No description provided.