[{Shi Zheng}] iP#258
Conversation
BigDoot
left a comment
BigDoot
left a comment
There was a problem hiding this comment.
There isn't very many issues with naming guidelines at the time of this review(as there aren't many methods yet), other than a few minor nitpicks! Fix them and you are good to go!
| System.out.println("Bye. Hope to see you again soon!"); | ||
| } | ||
|
|
||
| public static void errorHandling(String input) throws DukeException { |
There was a problem hiding this comment.
Following the coding guidelines, method names should be verbs. Perhaps handleError() would be a better method name?
| return this.description; | ||
| } | ||
|
|
||
| public Task changeStatus(Task t) { |
There was a problem hiding this comment.
Since this method is a setter, maybe setStatusTrue() would be better? It would explain to readers what status you are changing to as well!
| super(description); | ||
| } | ||
|
|
||
| public Deadline(String description, boolean b) { |
There was a problem hiding this comment.
instead of using b, you could just use isDone.
| /** | ||
| * main method to run the program | ||
| * | ||
| * @param args command line arguments taken in | ||
| */ |
There was a problem hiding this comment.
Minor nitpick but perhaps consider starting Javadoc sentences with an uppercase letter and ending with a full stop to keep within the coding standard.
| int taskToDelete = Integer.parseInt(input.substring(7)); | ||
| Task toRemove = items.get(taskToDelete - 1); |
There was a problem hiding this comment.
Perhaps int taskToDelete could be indexOfTaskToDelete even though it may be quite long.
It may help to distinguish it from Task toRemove.
hengyiqun
left a comment
hengyiqun
left a comment
There was a problem hiding this comment.
Clear code overall. Just a few nits to work out.
| } | ||
|
|
||
| public Event(String description, boolean b) { | ||
| super(description,b); |
There was a problem hiding this comment.
Perhaps you could use isDone instead of b, so that it is more descriptive?
| } | ||
|
|
||
| public Deadline(String description, boolean b) { | ||
| super(description,b); |
There was a problem hiding this comment.
For greater readability, perhaps you could have a whitespace after the comma?
| @@ -0,0 +1,14 @@ | |||
| public class Event extends Task { | |||
| public Event(String description) { | |||
There was a problem hiding this comment.
Should there be an empty line before this constructor?
I notice that there are times you leave an empty line before constructors, and there are times you leave out the empty line.
| System.out.println("Bye. Hope to see you again soon!"); | ||
| } | ||
|
|
||
| public static void errorHandling(String input) throws DukeException { |
OOP and Jar
Use check style
Inspired by https://github.com/sc-arecrow/ip.
A better gui
Inspired by https://github.com/sc-arecrow/ip.
A better Gui new
Added userguide
A user guide
Added userguide updates v2
A code quality
5 participants
No description provided.