[Huang Zhenxin] ip #249
[Huang Zhenxin] ip #249
Conversation
…type property to Task.java
| public class AddCommand extends Command { | ||
|
|
||
| /** | ||
| * Create a AddCommand object for execution. |
There was a problem hiding this comment.
Perhaps it is 'Creates' instead of 'Create'?
There was a problem hiding this comment.
... I noticed the same minor grammatical issue in other places too
|
|
||
|
|
||
| private static Boolean handleDeadline(String task, String date) { | ||
|
|
There was a problem hiding this comment.
Should this new line be removed?
|
|
||
|
|
There was a problem hiding this comment.
Should these lines between class declaration and constructor be removed?
| public class DeleteCommand extends Command { | ||
|
|
||
|
|
||
| public DeleteCommand(String task, String date) { |
There was a problem hiding this comment.
Perhaps Javadoc comments could be added for the constructor?
| /** | ||
| * Sub-class of command that represents and execute the "bye" instruction of user. | ||
| */ | ||
| public class ExitCommand extends Command { |
There was a problem hiding this comment.
I like how you further extract Command into child classes :)
|
|
||
| import duke.driver.DukeDriver; | ||
|
|
||
|
|
| TaskList.addTask(deadlines); | ||
| System.out.println(Ui.biggerBox(deadlines)); | ||
| } | ||
|
|
There was a problem hiding this comment.
Some of the new line spacing is a bit inconsistent. Perhaps you can standardize whether you're putting a new line spacing after every curly brace
| * @param date date of the user task to be done. | ||
| */ | ||
| public FindCommand(String task, String date) { | ||
| super("find", task, date, command -> { |
There was a problem hiding this comment.
I like how you split up the code into separate lines! Much more readable
| /** | ||
| * sub-class of Task to represents a task with deadline. | ||
| */ | ||
| public class Deadlines extends Task { |
There was a problem hiding this comment.
Perhaps this can be named Deadline instead of Deadlines, since it is a single Task
| } | ||
| } | ||
|
|
||
| public static void clearAllTask() { |
There was a problem hiding this comment.
Shouldn't this be clearAllTasks instead?
src/main/java/duke/ui/Ui.java
Outdated
| * @param t task to be wrapped. | ||
| * @return the String representation of the task name wrapped in a chatBox. | ||
| */ | ||
| public static String biggerBox(Task t) { |
There was a problem hiding this comment.
Perhaps can rename this to makeBiggerBox
| * @param date the String representation of date. | ||
| * @return MMM d yyyy format of date and return error message if the given date is in wrong format. | ||
| */ | ||
| public static String converter(String date) { |
There was a problem hiding this comment.
Should name this as an action, like convert
| @@ -0,0 +1,103 @@ | |||
| package duke.ui; | |||
There was a problem hiding this comment.
I like how everything is packaged! Very neat and clear :)
branch-A-Assertions
Improve code quality
No description provided.