[Ng Song Guan] iP #46
Conversation
| } | ||
|
|
||
| public static void main(String[] args) { | ||
| Duke duke = new Duke("./data/duke.txt"); |
There was a problem hiding this comment.
Maybe it will be better to make the path ./data/duke.text a constant?
src/main/java/duke/util/Parser.java
Outdated
| case "bye": | ||
| return new ByeCommand(); | ||
| case "list": | ||
| return new ListCommand(); | ||
| case "todo": | ||
| case "deadline": | ||
| case "event": | ||
| if (splitString.length == 1) { | ||
| throw new InvalidDescriptionException(); | ||
| } | ||
| return parseTask(command, splitString[1].trim()); | ||
| case "done": | ||
| num = Integer.parseInt(splitString[1]); | ||
| return new DoneCommand(num); | ||
| case "delete": | ||
| num = Integer.parseInt(splitString[1]); | ||
| return new DeleteCommand(num); | ||
| case "find": | ||
| String matchString = splitString[1].trim(); | ||
| return new FindCommand(matchString); | ||
| default: | ||
| throw new InvalidCommandException(); |
There was a problem hiding this comment.
Maybe you can declare the strings as constants? For example private static final String BYE = "bye", and use case BYE:
src/main/java/duke/Duke.java
Outdated
| * Starts execution of the Duke program. | ||
| */ | ||
| public void run() { | ||
| ui.printHello(); |
There was a problem hiding this comment.
Why did you choose to use ui.printHello() instead of printHello() (the method right above) ?
| import duke.task.Task; | ||
| import duke.util.TaskList; | ||
| import duke.util.Ui; | ||
|
|
There was a problem hiding this comment.
Might be better to add header comments here?
src/main/java/duke/Duke.java
Outdated
| public String getResponse(String input) { | ||
| try { | ||
| Command c = Parser.parse(input); | ||
| String res = c.execute(tasks, ui); |
There was a problem hiding this comment.
Maybe you can give a better variable name? Such as response or result?
|
|
||
| private Scanner scanner; | ||
|
|
||
| public Ui() { |
There was a problem hiding this comment.
Perhaps you can add java docs for this method?
|
|
||
| private List<Task> store; | ||
|
|
||
| public TaskList(List<Task> lst) { |
There was a problem hiding this comment.
Maybe can add java docs for this method as well.
src/main/java/duke/util/Parser.java
Outdated
| LocalDate date; | ||
| try { | ||
| switch (command) { | ||
| case "todo": |
There was a problem hiding this comment.
Good job following the module's coding style of indentation for the switch case!
| this.matchString = matchString; | ||
| } | ||
|
|
||
| public String execute(TaskList taskList, Ui ui) { |
There was a problem hiding this comment.
Perhaps a bit lengthy code here? Not sure if some parts could be abstracted out of the execute method......
Add assertions
Branch a code quality
Use lambda for list command
Use streams for find command
No description provided.