[Tan Ying Hui] iP #253
[Tan Ying Hui] iP #253
Conversation
… kept ruining the tests
Used exceptions and if, else and print statements to signal and display error Reformatted some of the if-else code to switch cases Figured out a better way to split the input
Handled errors such as insufficient arguments in command, unknown commands, empty description for tasks Did regression testing and passed Added new test cases for error handling
Removed any white spaces in front/ behind the input Also passed the same test cases as before
Added the delete feature and handled the same type of errors handled previously Did regression testing and passed Added new delete test cases
Added a SaveLoad class that is responsible for loading and saving a local save file for PandaBot Handled error for any loading error in the SaveLoad class
Created a DateAndTime class which allows PandaBot to understand date and time in the format dd/mm/yyyy hhmm Edited the Deadline behaviour such that Deadlines can accept both formatted and unformatted input as due by dates
Added FindCommand feature
# Conflicts: # src/main/java/DeleteCommand.java # src/main/java/Event.java # src/main/java/PandaBot.java # src/main/java/PandaBotException.java # src/main/java/PandaBotInvalidArgumentFormatException.java # src/main/java/PandaBotInvalidCommandException.java # src/main/java/PandaBotLoadingTasksErrorException.java
Missed out checking the validity of the date input, which has been added
src/main/java/AddCommand.java
Outdated
| this.task = task; | ||
| } | ||
|
|
||
| /** |
src/main/java/ByeCommand.java
Outdated
| /** | ||
| * Represents a bye command which allows users to exit the program. | ||
| */ | ||
| public class ByeCommand extends Command { |
There was a problem hiding this comment.
It seems you are using different classes for UI output, it's okay.
You can also do this in one single class lah
There was a problem hiding this comment.
I thought it would be better if the Ui class was in charge of all the messages to be printed (Single Responsibility Principle?)
src/main/java/Command.java
Outdated
| * Represents a Command, which is an abstract class. | ||
| * The Command class is used to execute a command. | ||
| */ | ||
| public abstract class Command { |
There was a problem hiding this comment.
Oh you use abstract class, that's good.
src/main/java/FindCommand.java
Outdated
| * @throws PandaBotException If any errors occurs when executing the command | ||
| */ | ||
| @Override | ||
| public void execute(TaskList tasks, Ui ui, Storage storage) throws PandaBotException { |
There was a problem hiding this comment.
Maybe name this function as executeOrder/executeCommand would be better
There was a problem hiding this comment.
Your code is easy to follow and understand. I like how you separated the commands into the different command type as this allows for easy addition of more command type in the future and also customisation of each command execution. The structure of your program was good and really complies with OOP.
One suggestion that I would like to make is to use packaging as it helps structure and organises your project files, especially when your project files grow with each iteration.
Overall, the project is quite well done and a pleasure to read. All the best!! 👍
src/main/java/Deadline.java
Outdated
| private String dueBy; | ||
|
|
||
| /** | ||
| * Create a Deadline object. |
There was a problem hiding this comment.
Should it be "Creates a Deadline object" instead?
There was a problem hiding this comment.
Yes, thanks for pointing that out.
src/main/java/ByeCommand.java
Outdated
| /** | ||
| * Returns true if the program should exit. | ||
| * Otherwise, returns false if the program should continue to run. | ||
| * | ||
| * @return true as the program exits on bye command | ||
| */ |
There was a problem hiding this comment.
I think that the {@inheritdoc} might be helpful here if the JavaDoc of the overridden method is the same.
| /** | ||
| * Represents a type of PandaBotException which is thrown when the task description is empty. | ||
| */ | ||
| public class PandaBotEmptyTaskDescriptionException extends PandaBotException{ |
There was a problem hiding this comment.
Should there be whitespace before the curly braces?
There was a problem hiding this comment.
Yes, thanks for pointing that out.
src/main/java/Storage.java
Outdated
| } | ||
|
|
||
| private Task convertToTask(String input) throws PandaBotException{ | ||
| String[] tDes = input.split(" \\| "); |
There was a problem hiding this comment.
Perhaps a more intuitive variable name here? For example, taskDetails
src/main/java/Ui.java
Outdated
| int len = tasks.size(); | ||
| if (len == 0) { | ||
| System.out.println("WOOTS! You don't have any tasks to do at the moment."); | ||
| } else { | ||
| System.out.println("These are the task(s) you have: "); | ||
| int i = 0; | ||
| for (Task t : tasks.getTaskList()) { | ||
| if (t != null) { | ||
| System.out.println((i + 1) + ". " + t.toString()); | ||
| i++; | ||
| } else { | ||
| break; | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Just a thought of mine but perhaps these implementations could be handled by their respective command and Ui could purely be in charge of printing out the String outputs given by the execution of the commands
There was a problem hiding this comment.
I realised your suggestion came in handy, for the GUI implementation. Thanks!
Currently, no assertions are used. Assertions can be used to verify any assumptions made while coding. This differs from using exceptions to check external inputs. Use assertions to the code verify that any assumptions made holds true.
Some parts of the code do not follow the recommended code quality guidelines. Code should be of high quality in order to maximise readibility. Improve code quality by following the recommended guidelines.
Some of the method and variable names are not well-named. This could lead to potential confusion. Use proper names and use names to explain the code to improve code quality.
Implement A-Assertions
# Conflicts: # src/main/java/pandabot/parser/Parser.java # src/main/java/pandabot/storage/Storage.java
Improve Code Quality
This gives PandaBot more features. An additional change is: Extracting a parent class, TimedTask. Tasks such as Event, Deadline, and the newly added DoAfter have a common attribute - time. Processing the various tasks with time separately causes code duplication. Let's extract the time attribute into a new parent class named TimedTask, which can be used to represent tasks with time attribute. It can accept both unformatted and formatted time input.
PandaBot does not have any means to allow users to be sure that they are giving the correct input format. This could confuse users who are not familiar with PandaBot's features. Add a help command which will bring up a help page that contains a list of commands that the user can use. Let's make this help page to help familiarise users with PandaBot's available commands.
The PandaBot GUI is very basic. It can be improved by personalising the GUI. Let's * Make the GUI window responsive to resizing of window * Add a background to PandaBot using CSS * Stylize the dialog box using CSS
No description provided.