[Choo Jia Xin] iP #318
[Choo Jia Xin] iP #318
Conversation
This reverts commit c1b819c.
src/duke/task/Task.java
Outdated
| private boolean isDone; | ||
|
|
||
| /** | ||
| * Creates a new task with the specified description. |
There was a problem hiding this comment.
JDoc guidelines suggest we should phrase constructors is like so :
Constructs....
There was a problem hiding this comment.
Code Standard wise:
Overall, not many code standard violations occured, good job for that.
Documentation was very well done at most parts, except for some constructors.
Couldnt nit pick very much for naming violations.
On the side of design: May be better to refactor to reduce the number of static methods used in the code base.
Good job overall!
src/main/java/duke/Duke.java
Outdated
| */ | ||
| private void run() { | ||
| ui.loadTaskList(tasks); | ||
| ui.showGreetings(); |
There was a problem hiding this comment.
This is a minor suggestion based on personal preference. Maybe use 'print' instead of 'show'?
| * | ||
| * @return Total number of tasks currently in the task list. | ||
| */ | ||
| public int totalNumberOfTasks() { |
There was a problem hiding this comment.
Perhaps add 'get' to the method name, i.e. getTotalNumberofTasks()?
| * @param index Index of task to be retrieved. | ||
| * @return Task stored at the specified index. | ||
| */ | ||
| public Task getTask(int index) { |
There was a problem hiding this comment.
This is just a personal preference too; maybe can add 'AtIndex' to the method name i.e. getTaskAtIndex.
| * | ||
| * @param index Index of the task to be deleted. | ||
| */ | ||
| public void deleteTask(int index) { |
There was a problem hiding this comment.
Same as above regarding the addition of 'AtIndex'
| * | ||
| * @return true if the task is done and returns false if the task is not done. | ||
| */ | ||
| public boolean hasDoneStatus() { |
There was a problem hiding this comment.
Same as above regarding the addition of 'get' word to the method name i.e. getHasDoneStatus() or rather getIsDone()
Merge branch-A-Assertions
Merge branch 'branch-A-CodeQuality'
No description provided.