tcjazwei iP #508
tcjazwei iP #508
Conversation
| */ | ||
| public Task(String name) { | ||
| this.taskName = name; | ||
| this.isDone = false; |
There was a problem hiding this comment.
Try to reduce the use of this. Use of this is discouraged, potentially to avoid unnecessary noise in the code. Try to avoid using this except when it is shadowed by a method or constructor parameter.
| System.out.println(LOGO); | ||
| System.out.println(VERSION); | ||
| System.out.println(LINE); | ||
| System.out.println(); |
There was a problem hiding this comment.
You could extract these print statements into a separate method, to follow the Single Layer of Abstraction Principle (SLAP)
There was a problem hiding this comment.
Overall, I like the personalization of DACKEL. I noted some features that could be extracted out so that Dackle does not have to handle everything, this should make the code easier to debug when there are expansions. Also, using String,format can help in making the code more readable and easier to format.
| + "( _ \\ /__\\ / __)( )/ )( ___)( ) \n" | ||
| + " )(_) )/(__)\\( (__ ) ( )__) )(__ \n" | ||
| + "(____/(__)(__)\\___)(_)\\_)(____)(____) "; | ||
| private static final String LINE = "____________________________________________________________"; | ||
| private static final String VERSION = "Version 0.1"; |
There was a problem hiding this comment.
I like that the programme is personalized to output DACKEL.
| private static void addTodo(String taskName) { | ||
| Todo newTask = new Todo(taskName); | ||
| storedTasks.add(newTask); | ||
| numberOfTasks++; | ||
| speak("added the following task to your list!\n " + newTask.toString()); | ||
| speak("your list now has " + String.valueOf(numberOfTasks) + " tasks."); | ||
| } | ||
|
|
||
| /** | ||
| * Adds a Deadline to storedTasks | ||
| * | ||
| * @param taskName name of task to be added | ||
| * @param dueTime due date/time of task as a String | ||
| */ | ||
| private static void addDeadline(String taskName, String dueTime) { | ||
| Deadline newTask = new Deadline(taskName, dueTime); | ||
| storedTasks.add(newTask); | ||
| numberOfTasks++; | ||
| speak("added the following task to your list!\n " + newTask.toString()); | ||
| speak("your list now has " + String.valueOf(numberOfTasks) + " tasks."); | ||
| } | ||
|
|
||
| /** | ||
| * Adds an Event to storedTasks | ||
| * | ||
| * @param taskName name of task to be added | ||
| * @param startTime starting date/time of task as a String | ||
| * @param endTime ending date/time of task as a String | ||
| */ | ||
| private static void addEvent(String taskName, String startTime, String endTime) { | ||
| Event newTask = new Event(taskName, startTime, endTime); | ||
| storedTasks.add(newTask); | ||
| numberOfTasks++; | ||
| speak("added the following task to your list!\n " + newTask.toString()); | ||
| speak("your list now has " + String.valueOf(numberOfTasks) + " tasks."); | ||
| } | ||
|
|
||
| /** | ||
| * Removes task with specified index from storedTasks | ||
| * | ||
| * @param index index of the element to be removed | ||
| */ | ||
| private static void deleteTask(int index) { | ||
| Task taskToBeRemoved = storedTasks.get(index); | ||
| String s = "the following task will be deleted from your list:\n "; | ||
| s += taskToBeRemoved.toString(); | ||
| speak(s); | ||
| speak("are you sure you want to do this? [Y/n]: "); | ||
| String confirmation = receiveInput(); | ||
| if (confirmation.equals("Y")) { | ||
| storedTasks.remove(index); | ||
| numberOfTasks--; | ||
| speak("task " + taskToBeRemoved.toString() + " successfully deleted."); | ||
| speak("you now have " + String.valueOf(numberOfTasks) + " tasks on your list."); | ||
| } | ||
| else { | ||
| speak("task was not deleted."); | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Lists all tasks in storedTasks. Displays a message if storedTasks is empty | ||
| */ | ||
| private static void listTasks() { | ||
| if (numberOfTasks == 0) { | ||
| speak("you have no tasks on your list!"); | ||
| return; | ||
| } | ||
| String s = ""; | ||
| for (int i = 0; i < numberOfTasks; i++) { | ||
| s += "\n" + String.format(" %2d " + storedTasks.get(i).toString(), i + 1); | ||
| } | ||
| speak(s); | ||
| } | ||
|
|
||
| /** | ||
| * Marks the specified task as done | ||
| * | ||
| * @param index index of task in storedTasks | ||
| */ | ||
| private static void markTask(int index) { | ||
| storedTasks.get(index).mark(); | ||
| speak("i've marked the following task as done!\n " + storedTasks.get(index).toString()); | ||
| } | ||
|
|
||
| /** | ||
| * Marks the specified task as not done | ||
| * | ||
| * @param index index of task in storedTasks | ||
| */ | ||
| private static void unmarkTask(int index) { | ||
| storedTasks.get(index).unmark(); | ||
| speak("i've unmarked the following task. do it soon, please!\n " + storedTasks.get(index).toString()); | ||
| } |
There was a problem hiding this comment.
Should this be extracted out?
I noticed Dackel is used to handle everything from creating tasks to handling errors but I do think it will be easier to debug if you can extract the functionalities into different classes. I suggest having a parser class to parse strings to split up the task creation instead of letting Dackel handle everything.
| @Override | ||
| public String toString() { | ||
| String s = "[E]" + super.toString(); | ||
| s = s + " (from " + this.startTime + " "; | ||
| return s + "until " + this.endTime + ")"; | ||
| } |
There was a problem hiding this comment.
I suggest using String.format to have an easier time producing the string as you can type it out as a sentence that is close to the output.
| private static void unmarkTask(int index) { | ||
| storedTasks.get(index).unmark(); | ||
| speak("i've unmarked the following task. do it soon, please!\n " + storedTasks.get(index).toString()); | ||
| } |
There was a problem hiding this comment.
I think splitting up the functionality of this class into separate classes would be better? That way, implementing new functionality would be easier and it is also easier to debug if there are errors.
| String s = "[D]" + super.toString(); | ||
| return s + " (due by " + this.dueTime + ")"; | ||
| } | ||
| } |
There was a problem hiding this comment.
For the deadline input, consider adding in the functionality to take in date time inputs.
| System.out.println(LINE); | ||
| System.out.println(LOGO); | ||
| System.out.println(VERSION); | ||
| System.out.println(LINE); | ||
| System.out.println(); |
There was a problem hiding this comment.
For the print statements, create a separate method.
No description provided.