[sweetFish8] iP#189
Conversation
changed logo renamed filename
This reverts commit fc8da49.
|
|
||
| @Override | ||
| public String toString() { | ||
| return "[E]" + super.toString() + " (from: " + from + " to: " + to + ")"; |
There was a problem hiding this comment.
Perhaps use a variable to store [E]/[T]/[D] literals?
| } | ||
|
|
||
| private static void printTaskList(Task[] tasks, int taskCount) { | ||
| System.out.println(" ____________________________________________________________"); |
There was a problem hiding this comment.
Maybe assign this partition to a final string variable to use multiple times?
| public static void main(String[] args) { | ||
| Scanner scanner = new Scanner(System.in); | ||
|
|
||
| String logo = |
There was a problem hiding this comment.
Interesting logo! But seems a bit messy.
tongkiankiat
left a comment
tongkiankiat
left a comment
There was a problem hiding this comment.
The code looks neat and well done, with good use of methods to handle different cases. Good job!
| if (taskNumber >= 0 && taskNumber < taskCount) { | ||
| if (isMark) { | ||
| tasks[taskNumber].markAsDone(); | ||
| System.out.println(" ____________________________________________________________"); |
There was a problem hiding this comment.
Could store this string in a variable to reduce character space as its being reused constantly
| + " _____________________________" | ||
| + "_______________________________\n" | ||
| + "\n" | ||
| + logo); |
There was a problem hiding this comment.
Good practice storing the large text display in a variable and not type it again to prevent excessive lines of code
| this.to = to; | ||
| } | ||
|
|
||
| @Override |
There was a problem hiding this comment.
Good practice including the @ Override tag when overriding a method.
|
|
||
| @Override | ||
| public String toString() { | ||
| return "[T]" + super.toString(); |
There was a problem hiding this comment.
Since the [T], [D], [E] strings are appearing multiple times, maybe could store them in a variable?
…nd change the expected text to updated one
thomasjlalba
left a comment
thomasjlalba
left a comment
There was a problem hiding this comment.
Good job and like the personalisation of your iP. Just some small suggestions to improve code quality
| String details = commandParts.length > 1 ? commandParts[1].trim() : ""; | ||
|
|
||
| switch (command) { | ||
| case "bye": |
There was a problem hiding this comment.
Maybe you can avoid magic literals.
| if (details.isEmpty()) | ||
| throw new SparkleException( | ||
| SparkleException.ErrorType.EMPTY_TASK_DESCRIPTION, "Event"); | ||
| String[] eventParts = details.split(" /from ", 2); | ||
| if (eventParts.length < 2 || !eventParts[1].contains(" /to ")) { | ||
| throw new SparkleException( | ||
| SparkleException.ErrorType.INVALID_FORMAT, "Event requires /from and /to time."); | ||
| } | ||
| String[] timeParts = eventParts[1].split(" /to ", 2); | ||
| tasks[taskCount] = | ||
| new Event(eventParts[0].trim(), timeParts[0].trim(), timeParts[1].trim()); | ||
| printAddedTask(tasks[taskCount++], taskCount); | ||
| break; | ||
|
|
||
| default: | ||
| throw new SparkleException(SparkleException.ErrorType.UNKNOWN_COMMAND, command); | ||
| } |
There was a problem hiding this comment.
Maybe you can look into abstracting the cases into their own methods to make it less cluttered
| printTaskList(tasks, taskCount); | ||
| break; | ||
|
|
||
| case "mark": |
There was a problem hiding this comment.
Maybe you can explicitly state // Fallthrough to indicate it is intentional as per coding standards
| if (details.isEmpty()) | ||
| throw new SparkleException(SparkleException.ErrorType.EMPTY_TASK_DESCRIPTION, "Todo"); | ||
| tasks[taskCount] = new Todo(details); | ||
| printAddedTask(tasks[taskCount++], taskCount); |
There was a problem hiding this comment.
Maybe you can look to avoid complicated expressions in your code, for example separating the taskCount++ to another line to improve readability.
This reverts commit 914953f.
Add Javadoc to task and subclasses
Find method to search task
Add Javadoc to all classes
4 participants
No description provided.