[Chiam Yun Qing] iP #22
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…s easier, Add storage class for handling saving and loading of data
* branch-Level-7: Create new data folder to store duke.txt Add functionality for saving task and loading task from hard disk Update string format of task classes to make handling of loading tasks easier, Add storage class for handling saving and loading of data
* branch-Level-8: Add command to enable printing of tasks occurring on a specific date Add feature to process date and time for events and deadlines # Conflicts: # src/main/java/Deadline.java # src/main/java/Duke.java # src/main/java/Event.java
* branch-A-Level-10: Complete functionality for hi and bye messages
thutahw
reviewed
Sep 1, 2020
| + "I assume that you are as forgetful as Nobita. Remember to bribe me with loads of Dorayaki too!"; | ||
| return intro; | ||
| } | ||
|
|
There was a problem hiding this comment.
Would it be better to name this method as "showWelcomeMessage"?
| String bye = "Goodbye, I need to find my sister now! >_<"; | ||
| return bye; | ||
| } | ||
|
|
There was a problem hiding this comment.
Would it be better if "showByeMessage" is used instead?
| listOfTask.add(new Todo("study")); | ||
| assertEquals(2, new TaskList(listOfTask).getNumTasks()); | ||
| } | ||
|
|
There was a problem hiding this comment.
Maybe add a spacing here: "testGetNumTask() {" ? Also, would "listOfTasks" look better?
| assertEquals(listOfTask, new TaskList(listOfTask).getSameDateTasks("2020-08-08")); | ||
| assertEquals(new ArrayList<Task>(), new TaskList(listOfTask).getSameDateTasks("2020-08-09")); | ||
| } | ||
|
|
There was a problem hiding this comment.
I think it's the same as above, just missing 1 spacing between ")" and "{".
| assertEquals(listOfTask, new TaskList(new ArrayList<Task>(listOfTask)).getAllTasks()); | ||
| assertEquals(new ArrayList<Task>(), new TaskList().getAllTasks()); | ||
| } | ||
| } |
There was a problem hiding this comment.
Everything is good! Maybe just the missing spacing like above.
Most issues arise from user mistakes and have already been handled by exceptions
Add assertions to make sure objects are present at those states
Improve code quality (e.g. comply with SLAP, prominent happy path)
Use streams in TaskList
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.