[Ming Yang] iP #216
[Ming Yang] iP #216
Conversation
A-CodingStandard
Add Gradle support
| private boolean end; | ||
|
|
||
| /** | ||
| * Initializes a Command object |
There was a problem hiding this comment.
Consider ending the summary sentence with a period?
Javadoc requires that the first sentence ends with a period. https://www.oracle.com/technical-resources/articles/java/javadoc-tool.html
| * @param task String representing task to be appended | ||
| */ | ||
|
|
||
| public void appendFile (String task) { |
There was a problem hiding this comment.
Consider removing the whitespace between the method name and parameters?
src/main/java/duke/TaskList.java
Outdated
| int size = todo.size() - 1; | ||
| Ui.showLine(); | ||
| System.out.println("Noted. I've removed this task: \n" | ||
| + todo.get(taskNumber - 1) + "\n" + "Now you have " + size + " task in the list \n" |
There was a problem hiding this comment.
This is slightly over 110 characters. Since it's already being wrapped from Line 54 maybe you could wrap it again to another line?
src/main/java/duke/TaskList.java
Outdated
| * Adds an event task to the task list. | ||
| * | ||
| * @param line the line in which the command of adding event was given. | ||
| * @throws DukeInvalidTaskException |
There was a problem hiding this comment.
Consider specifying under what conditions the exception is thrown?
src/test/java/DeadlineTest.java
Outdated
| Deadline task = new Deadline("hi", "2020-01-01"); | ||
| String expected = "[D][✗] hi (by: Jan 1 2020)"; | ||
| assertEquals(expected, task.toString()); | ||
| } catch (DukeInvalidTaskException | DukeInvalidDateException e) { |
There was a problem hiding this comment.
If an exception is caught and you weren't expecting it, will this cause the test to pass mistakenly? You might want to check that out.
src/main/java/duke/Command.java
Outdated
| * @param action The type of action take as given by the task. | ||
| */ | ||
| public Command(String task, String action) { | ||
| this.end = false; |
There was a problem hiding this comment.
Maybe leave out the this keyword when it's not needed to make the code more succinct?
src/main/java/duke/Command.java
Outdated
| * @param ui the user interface object. | ||
| */ | ||
|
|
||
| public void execute(TaskList taskList, Ui ui) throws DukeException { |
There was a problem hiding this comment.
This method is quite long. Perhaps you could abstract away parts to make it more readable?
src/main/java/duke/Storage.java
Outdated
| public class Storage { | ||
| private final File dataFile; | ||
| @SuppressWarnings({"checkstyle:AbbreviationAsWordInName", "CheckStyle"}) | ||
| private final String FILE_PATH; |
There was a problem hiding this comment.
Perhaps use camel casing here, since file path is not a constant, but varies between instances of the Storage class?
src/main/java/duke/Storage.java
Outdated
| FileWriter fw = new FileWriter(FILE_PATH); | ||
| for (Task currentTask : list) { | ||
| String done = currentTask.isDone() ? "1" : "0"; | ||
| if (currentTask instanceof Event) { |
There was a problem hiding this comment.
Perhaps you could make use of polymorphism here, since all 3 task types extends the Task class. This can help make the code a lot more succinct and readable.
src/main/java/duke/Ui.java
Outdated
| * | ||
| * @param list in which the tasks are stored | ||
| */ | ||
| public void welcomeMessage(TaskList list) { |
There was a problem hiding this comment.
Maybe naming the method with a verb can help the program be more understandable?
|
|
||
| public Deadline(String taskName, String date) throws DukeInvalidDateException, DukeInvalidTaskException { | ||
| super(taskName); | ||
| if (!date.equals(null) && !date.equals(" ")) { |
There was a problem hiding this comment.
Maybe using guard clauses here can ensure that your main program logic is more prominent?
cleanup unused code
Branch-A-Assertions
No description provided.