[Gregg] iP - #509
Conversation
# Conflicts: # src/main/java/Duke.java
ee-suan
left a comment
ee-suan
left a comment
There was a problem hiding this comment.
Great job, I think the code is easy to read
| * @param bool The boolean for whether the task is marked | ||
| * or unmarked. | ||
| */ | ||
| public void isMark(boolean bool) { |
There was a problem hiding this comment.
Maybe this method name should start with a verb?
| @@ -0,0 +1,14 @@ | |||
| package exception; | |||
|
|
|||
| public class DukeException extends IllegalArgumentException{ | |||
There was a problem hiding this comment.
Maybe a whitespace before the opening brace here?
| } | ||
| } | ||
|
|
||
| public void toPrint() { |
There was a problem hiding this comment.
Maybe this method name should start with a verb
| public class FileData { | ||
| protected String fileName; | ||
| protected File file; | ||
| protected ArrayList<Task> tasks; |
| } | ||
| } | ||
|
|
||
| public ArrayList<Task> allTasks() { |
There was a problem hiding this comment.
Maybe include a verb here for the method name eg. getAllTasks()
| public class DeadlineTest { | ||
|
|
||
| @Test | ||
| public void getStatusIcon_unmarked() { |
There was a problem hiding this comment.
I like the clarity in your naming of the test methods!
ElginL
left a comment
ElginL
left a comment
There was a problem hiding this comment.
Good job. Overall, most of the coding standards are complied with, except for a trivial amount that may have been accidentally left out. LGTM!
| System.out.println("Got it. I've added this task:\n" + | ||
| " " + currTask + "\n" + | ||
| "Now you have " + taskList.size() + " tasks in the list.\n"); |
There was a problem hiding this comment.
I think the styling of this would be better if the line breaks before an operator. I've noticed this quite a bit, but good job on using linebreaks 👍
| */ | ||
|
|
||
| public class AddCommand extends Command { | ||
| protected String commandLine; |
There was a problem hiding this comment.
I didn't like the variable name for this, perhaps userInput is more intuitive? 🤔
| * @return false. | ||
| */ | ||
| @Override | ||
| public boolean isExit() { |
There was a problem hiding this comment.
Good job on naming all your methods as a verb 👍
| @@ -0,0 +1,48 @@ | |||
| package parser; | |||
|
|
|||
| import command.*; | |||
There was a problem hiding this comment.
I didn't like the use of * in import because it violates the coding standard. It might be an improvement to list all imports explicitly
Use assert feature
Improve code quality
3 participants
No description provided.