[Siyan] iP #428
[Siyan] iP #428
Conversation
|
|
||
| public class Chatty { | ||
| private static final String linebreak = "___________________________________________________________________"; | ||
| private static final String introMsg = "\n\tHello Master! I'm Chatty, your ever-ready personal assistant.\n\tHow can I help you today?"; |
There was a problem hiding this comment.
I like how you created some constants here for some of the bots' messages. However, note that according to the Java Coding standard in this course, constants use the screaming snake case instead of the camel case used for normal variables.
| private static final String introMsg = "\n\tHello Master! I'm Chatty, your ever-ready personal assistant.\n\tHow can I help you today?"; | ||
| private static final String exitMsg = "\n\tGoodbye Master! See you soon!"; | ||
| private ArrayList<Task> taskList = new ArrayList<>(); | ||
| private Integer taskCounter = 0; |
There was a problem hiding this comment.
Any reason why you used the Wrapper class Integer instead of the primitive type int?
| public void handleCommand(String command) { | ||
| try{ | ||
| // handle list command | ||
| if (command.equalsIgnoreCase("list")) { |
There was a problem hiding this comment.
The other commands do not allow for case insensitivity. So why is the list command special? Maybe consider changing command to a single case (lower or upper) first before processing it if you want to allow for case insensitivity that is.
| public void deleteTask(String command) throws ChattyTaskNotFoundException, ChattyInvalidArgumentException { | ||
| String[] commandParts = command.split(" "); | ||
| if (commandParts.length == 2) { | ||
| int taskId = Integer.parseInt(commandParts[1].trim()); |
There was a problem hiding this comment.
Perhaps consider the case where the user types "delete study" where they didn't think of using the index at first. This would cause an NumberFormatException when trying to parse the 2nd word as a number which isn't handled.
| public void setTaskStatus(String command) throws ChattyTaskNotFoundException { | ||
| String[] commandParts = command.split(" "); | ||
| if (commandParts.length == 2) { | ||
| int taskId = Integer.parseInt(commandParts[1].trim()); |
There was a problem hiding this comment.
Same issue as the one mentioned for deleteTask. Perhaps consider placing the parsing of the index number in its own method since its being used in more than 1 place.
| } | ||
| } | ||
|
|
||
| public void handleTaskCreation(String command) throws ChattyInvalidArgumentException { |
There was a problem hiding this comment.
Perhaps the processing of the description string for each task can be handled in the respective Task subclasses instead?
| } | ||
| if (command.startsWith("event")) { | ||
| String taskString = command.substring(5).trim(); | ||
| String[] taskParts = taskString.split("/"); |
There was a problem hiding this comment.
What is the allowed format for event tasks here? If it is "name /from start /to end", the strings "from" and "to" are not checked, meaning the string "name /start start /end end" might be incorrectly parsed.
|
|
||
| @Override | ||
| public String getMessage() { | ||
| return String.format(" Invalid argument detected for command: %s", commandType); |
There was a problem hiding this comment.
Maybe informing the user of what the correct format should be can be useful.
There was a problem hiding this comment.
A lot of the methods used in the class seem to be called internally in this file alone. Perhaps consider making those that are not going to be called outside this class to be made private?
No description provided.