[jednghk] iP #320
[jednghk] iP #320
Conversation
src/main/java/TaskList.java
Outdated
| taskToMark.unmark(); | ||
| } | ||
|
|
||
| public String getTaskList() { |
There was a problem hiding this comment.
Just picking a nit but maybe you could consider renaming this method to getTaskListString. Might be less confusing in the future if you ever need a method to return the List<Task> taskList.
src/main/java/Duke.java
Outdated
| while (true) { | ||
| String userInput = sc.nextLine().toLowerCase(); | ||
| String[] command = userInput.split(" ", 10); | ||
| switch (command[0]) { |
There was a problem hiding this comment.
It's good that you adhered to the CS2103 java coding standards for switch statements.
src/main/java/TaskList.java
Outdated
| import java.util.List; | ||
|
|
||
| public class TaskList { | ||
| private List<Task> taskList = new LinkedList<>(); |
There was a problem hiding this comment.
Was there a particular reason why you chose to use a LinkedList<E> over an ArrayList<E>?
src/main/java/Duke.java
Outdated
| + "____________________________________________________________"); | ||
| } | ||
|
|
||
| private void greet() { |
There was a problem hiding this comment.
Would you consider encapsulating all of Duke's interactions with the user into a separate class? The class could be the Ui class, containing the methods greet(), printExitMessage(), etc.
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
For the project structure, I think it will be good to move Tasks (Deadline Todo...) to a package and move others to another.
src/main/java/duke/Storage.java
Outdated
| import java.io.*; | ||
| /** Handles the storage and retrieval of tasks. */ | ||
| public class Storage { | ||
| TaskList taskList; |
| Event event = (Event) this; | ||
| return String.format("%c|%b|%s|%s|%s", taskType, isDone, taskName, event.getFrom(), event.getTo()); | ||
| } | ||
| } |
There was a problem hiding this comment.
For this part, to use a switch-case block is concise
| public void unmarkTask(int taskIndex) { | ||
| Task taskToMark = this.getTask(taskIndex); | ||
| taskToMark.unmark(); | ||
| } |
There was a problem hiding this comment.
more comments expected for the methods above
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Also the main class should look more concise, the code inside the while loop could be extracted to Parser or another class.
Add more assertion statements
The getResponse() method body does not adhere to SLAP and has an arrowhead style code. This makes the code messier and harder to debug. Refactoring the method body will improve code quality. Let's create a Command class which handles the execution of the user input, and split the code into their respective methods to mitigate the arrowhead style code.
Abstract getResponse() method body
Duke
Have you ever wanted to:
Then this is the app for you!
As a software engineering project done by an inexperienced programmer, Duke can:
Record your todos, deadlines, events, but only when you follow a very specific format
That's about it!
Here's a checkbox for you to click for fun
If you're reading
this farfor some reason, here's a code block for you to analyse: