[Fahim Tajwar] iP #377
[Fahim Tajwar] iP #377
Conversation
…sted switch-case.
…, badly formatted deadlines, badly formatted events.
…xceptions to come.
There was a problem hiding this comment.
Good work here, some suggestions for your code:
- Wildcard imports shouldn't be used, this might've resulted from the default settings in Intellij that auto converts imports into wildcard imports (try disabling it)
- You might want to implement an
symbolattribute to each type of task, rather than hardcoding it into thetoString()method. This would also help for the storage requirement, if you want to use the task symbols in the storage format :) - A
reply(String reply)method in your Duke class can solve the tediousness of having to print the divider line for each output by the chatbot - Each case for a switch statement should be on the same indent line as the
switch():statement itself, there's also a setting in Intellij that disables that
Nice job though, I found your code easy to read and understand :)
Add error message for badly formatted dates.
src/main/java/Duke.java
Outdated
| ArrayList<Task> tasks = new ArrayList<>(); | ||
| Scanner sc = new Scanner(System.in); | ||
|
|
||
| while(sc.hasNext()) { |
There was a problem hiding this comment.
I noticed several Java reserved words such as while in this case not being followed by a white space. e.g.
| while(sc.hasNext()) { | |
| while (sc.hasNext()) { |
Perhaps you could consider using IntelliJ's code cleanup feature to add these white spaces automatically.
src/main/java/Duke.java
Outdated
| while(sc.hasNext()) { | ||
| String instr = sc.nextLine(); | ||
| switch(instr) { //Single-word instructions | ||
| case "list": //List down |
There was a problem hiding this comment.
Should indentation for case clauses be removed?
| case "list": //List down | |
| case "list": |
src/main/java/Duke.java
Outdated
| + "| |_| | |_| | < __/\n" | ||
| + "|____/ \\__,_|_|\\_\\___|\n"; | ||
| System.out.println("Hello from\n" + logo); | ||
| public static void main(String[] args) { |
There was a problem hiding this comment.
Should basic indentation be changed from 2 spaces to 4 spaces to align with the Java coding standard?
| public static void main(String[] args) { | |
| public static void main(String[] args) { |
src/main/java/Duke.java
Outdated
| ArrayList<Task> tasks = new ArrayList<>(); | ||
| Scanner sc = new Scanner(System.in); | ||
|
|
||
| while(sc.hasNext()) { |
There was a problem hiding this comment.
Be careful of using sc.hasNext() vs sc.hasNextLine()
src/main/java/Duke.java
Outdated
| System.out.println(divider + "OOPS!!! The description of a " + action + " cannot be empty.\n" + divider); | ||
| } | ||
|
|
||
| switch(action) { //For instructions with argument(s). |
There was a problem hiding this comment.
Would recommend against nesting switch-case block for code quality
src/main/java/Duke.java
Outdated
| } | ||
| } |
There was a problem hiding this comment.
A lot of nested blocks. Consider abstracting away with methods within the main class. For example you can have:
// in main
switch(...) {
case "todo":
addToDo(tokens);
}
// add implementation
public void addToDo(String[] tokens) {
// your implementation
}Add separate UI, Parser, TaskList classes. Add NoArgumentException to handle empty argument cases.
P.S. Checkstyle violations fixed in previous commits.
Long lists are no longer cut short with "...".
Remove redundant exception class. Remove dead code.
DukePro
DukePro frees your mind off remembering tasks. It's
FASTSUPER FAST to useAnd it's free!
All you need to do is:
todo,deadlineandevent.markandunmark.delete.find!Features: