[Tyrus Lye]iP #557
[Tyrus Lye]iP #557
Conversation
Not gonna lie. it doesnt work and i dont know why
src/main/java/Duke.java
Outdated
| String command = scanner.nextLine(); | ||
| System.out.println("____________________________________________________________"); | ||
| try{ | ||
| if (command.equalsIgnoreCase("bye")) { |
There was a problem hiding this comment.
would it be better to use enumerates for the commands?
src/main/java/Duke.java
Outdated
| } | ||
| } catch (FileNotFoundException e) { | ||
| // Handle file not found exception | ||
| System.out.println("File not found: " + FILE_PATH); |
There was a problem hiding this comment.
I like that the handling of this exception not only tells that user that the file cannot be found but also returns the filepath
src/main/java/Duke.java
Outdated
| for (int i = 0; i < tasks.size(); i++) { | ||
| System.out.println((i + 1) + ". " + tasks.get(i)); | ||
| } | ||
| } else if (command.startsWith("todo")) { |
There was a problem hiding this comment.
would it be better to package some of these if, else if else steps into a function that handles everything within the outer most if case? for eg. a function to handle creating todo here.
src/main/java/Duke.java
Outdated
| tasks.get(index).markDone(); | ||
| System.out.println("Nice! I've marked this task as done:\n " + tasks.get(index)); | ||
| } else { | ||
| System.out.println("Invalid task index."); |
There was a problem hiding this comment.
would it be useful if instead of printing invalid task index, the size of the tasklist is also returned in this else case to let user know that for example there is only 3 tasks in the tasklist thats why their input 4 is invalid. Something like "Invalid task index, the task list has " + tasklist.size() + "tasks. "
There was a problem hiding this comment.
LGTM overall!! The naming of the methods are all of the course standard, just that it might be better for you to start adding javadoc to your public methods and also take note of the indentation of switch statements. Readability can also be improved if commands are split into different methods to handle each case (instead of putting all in one method). Good job so far! :D
src/main/java/Duke.java
Outdated
| if (command.equalsIgnoreCase("bye")) { | ||
| System.out.println("Bye. Hope to see you again soon!"); | ||
| break; | ||
| } else if (command.equalsIgnoreCase("list")) { |
There was a problem hiding this comment.
Maybe it will be better if this part is separated into different functions like addToDoTask, etc. to improve readability, what do you think?
src/main/java/Duke.java
Outdated
| String description = parts[2]; | ||
|
|
||
| Task task = null; | ||
| switch (type) { |
There was a problem hiding this comment.
For switch statements, I think there shouldn't be indentation on the next line. On the mod website:
switch (condition) {
case ABC:
statements;
// Fallthrough
case DEF:
statements;
break;
case XYZ:
statements;
break;
default:
statements;
break;
}
src/main/java/Duke.java
Outdated
| } | ||
|
|
||
| @Override | ||
| public String toString() { |
src/main/java/Duke.java
Outdated
| Scanner scanner = new Scanner(System.in); | ||
| ArrayList<Task> tasks = new ArrayList<>(100); | ||
| loadTasksFromFile(tasks); | ||
| String logo = "██╗░░░██╗██████╗░██████╗░░█████╗░██╗\n" |
Fixed problem with Deadline not working and list not going to the next line
Fixing some formatting issues with displaying the list of tasks
A more oop
A-Assertions
No description provided.