[Christopher Goh] iP #223
[Christopher Goh] iP #223
Conversation
There was a problem hiding this comment.
Nicely abstracted and clean looking code. Can't spot any coding standard violations 🤣 I myself is also guilty of this but one possible suggestion moving forward would be to abstract the methods out of Duke.java so that the main file is shorter. (Perhaps you can consider having a Handler class) Keep up the good work!
| int taskNo; | ||
| try { | ||
| taskNo = Integer.parseInt(userInput.split(" ")[1]); | ||
| } catch (ArrayIndexOutOfBoundsException aioobe) { |
There was a problem hiding this comment.
Just a small suggestion, perhaps you can consider using a single letter 'e' to represent the exception but taking the first letter of every word is an interesting way to do it as well!
| System.out.println("ERROR: Date/time not found. Did you input a date/time with `/at`?"); | ||
| return; | ||
| } else if (input.length > 2) { | ||
| System.out.println("ERROR: Multiple date/times found. Please only input one date/time."); |
| return CommandType.EVENT; | ||
| case "find": | ||
| return CommandType.FIND; | ||
| case "rm": |
There was a problem hiding this comment.
Nice use of switch cases! It'll be good if u add a oneliner comment to briefly describe what rm is :)
| * @return tick or cross icon representation of the done status of the task | ||
| */ | ||
| public String getStatusIcon() { | ||
| return (isDone ? "\u2713" : "\u2718"); //return tick or X symbols |
There was a problem hiding this comment.
One small suggestion: I think you can remove the comment since you've already mentioned what this function returns in Javadocs.
| } | ||
|
|
||
| public static void main(String[] args) { | ||
| System.out.println("Hello I'm Duke!"); |
There was a problem hiding this comment.
One suggestion (after looking at AB2's code) would be to make this into a method though the current one is fine as well!
eg. UserInterface.showGreetingMessage();
refactor: Add java assertions
refactor: Remove unused imports
| public static void main(String[] args) { | ||
| Application.launch(Main.class, args); | ||
| } | ||
| } |
|
|
||
| private static List<Task> savedItems = loadSavedItems(); | ||
|
|
||
| private MainWindow mainWindowController; |
There was a problem hiding this comment.
not related to Code Standards, but may be good to break dependency of Duke on MainWindow though some level of abstraction.
|
|
||
| import java.io.Serializable; | ||
|
|
||
| public class Task implements Serializable { |
There was a problem hiding this comment.
Oh snap TIL this exists! Thanks for showing this.
| import java.time.format.DateTimeFormatter; | ||
| import java.time.format.DateTimeParseException; | ||
|
|
||
| public class Deadline extends Task { |
There was a problem hiding this comment.
Consider abstracting deadline and event into a TimedTask abstract class. This allows Event to also have synchronous functionalities.
| } catch (ArrayIndexOutOfBoundsException aioobe) { | ||
| return "ERROR: No task no. found. Did you input the task no. of the task you'd like to mark as done?"; | ||
| } catch (NumberFormatException nfe) { | ||
| return "ERROR: Unrecognized task. Please input the task no. of the task you'd like to mark as done."; |
There was a problem hiding this comment.
Would be good to encapsulate errors into a DukeException Error class. Perhaps even to store a Enum for generic messages like:
"You input: %s\n
Unrecognized task. Please input the task no. of the task you'd like to mark as done."
No description provided.