[DarkDragoon2002] iP#95
Conversation
1. Renamed Duke to Franz 2. Created the greeting and exit functions 3. Implemented the lines in between the messages
1. Cleaned up Code 2. Reduced Nesting 3. Added Comments
hzxnancy
left a comment
There was a problem hiding this comment.
Overall, the code looks neat with the extraction of different functions and the use of constructors. The personalisation is very unique which makes this code fun to read, gran trabajo!
However, do take note of the naming of boolean variables.
| helloMessage(); | ||
| lineMessage(); | ||
|
|
||
| boolean continueChatting = true; |
There was a problem hiding this comment.
Boolean variables/methods should be named to sound like booleans.
The same error is present in other parts of the code.
| // Less efficient to create a new scanner everytime but code is much neater | ||
| // If return True means continue | ||
| // Else End |
There was a problem hiding this comment.
This comment looks like a note to self and should not be here
| boolean continueChatting = true; | ||
| while (continueChatting) { | ||
| continueChatting = chatFeature(); | ||
| } |
There was a problem hiding this comment.
Would it be better to extract these lines of code?
| " ._-'-_ .\n" + | ||
| " . ' /_-_-_\\ ` .\n" + | ||
| " .' |-_-_-_-| `.\n" + | ||
| " ( `.-_-_-.' )\n" + | ||
| " !`. .'!\n" + | ||
| " ! ` . . ' !\n" + | ||
| " ! ! ! ! ! ! ! ! !\n" + | ||
| " / / \\ \\\n" + | ||
| " _-| \\___ ___/ /-_\n" + | ||
| " (_ )__\\_)\\(_/__( _)\n" + | ||
| " ))))\\X\\ ((((\n" + | ||
| " \\/ \\/ \n" + |
| String greeting = | ||
| " ._-'-_ .\n" + | ||
| " . ' /_-_-_\\ ` .\n" + | ||
| " .' |-_-_-_-| `.\n" + | ||
| " ( `.-_-_-.' )\n" + | ||
| " !`. .'!\n" + | ||
| " ! ` . . ' !\n" + | ||
| " ! ! ! ! ! ! ! ! !\n" + | ||
| " / / \\ \\\n" + | ||
| " _-| \\___ ___/ /-_\n" + | ||
| " (_ )__\\_)\\(_/__( _)\n" + | ||
| " ))))\\X\\ ((((\n" + | ||
| " \\/ \\/ \n" + |
| if (line.equals("bye")) { | ||
| return false; | ||
| } else if (line.equals("list")) { | ||
| Task.printTasksList(); | ||
| return true; | ||
| } | ||
|
|
||
| // Check for mark and unmark or just add task | ||
| String[] parts = line.split(" "); | ||
| if (parts[0].equals("mark")){ | ||
| // Mark | ||
| int taskIndex = Integer.parseInt(parts[1]) - 1; | ||
| Task.mark(taskIndex); | ||
| } else if (parts[0].equals("unmark")){ | ||
| // Unmark | ||
| int taskIndex = Integer.parseInt(parts[1]) - 1; | ||
| Task.unmark(taskIndex); | ||
| } else { | ||
| // else add task | ||
| Task newTask = new Task(line); | ||
| } | ||
|
|
|
|
||
| // Keep track of tasks | ||
| private static Task[] tasks = new Task[100]; | ||
| private static int taskNumber = 0; |
There was a problem hiding this comment.
Perhaps a better name can be made here, like taskCount?
|
|
||
| // Object specific variables | ||
| public String taskString; | ||
| public boolean taskDone = false; |
There was a problem hiding this comment.
Should the boolean name here be more intuitive like in the Java coding standard?
| public static void lineMessage() { | ||
| String line = "____________________________________________________________\n"; | ||
| System.out.print(line); | ||
| } |
There was a problem hiding this comment.
For the messages, should it be static variables instead and what the methods do is just printing it out?
| String line = scanner.nextLine(); | ||
| lineMessage(); | ||
|
|
||
| if (line.equals("bye")) { |
There was a problem hiding this comment.
May the command words also be variables, for later use or easier management?
| public static void helloMessage() { | ||
| String greeting = | ||
| " ._-'-_ .\n" + | ||
| " . ' /_-_-_\\ ` .\n" + | ||
| " .' |-_-_-_-| `.\n" + | ||
| " ( `.-_-_-.' )\n" + | ||
| " !`. .'!\n" + | ||
| " ! ` . . ' !\n" + | ||
| " ! ! ! ! ! ! ! ! !\n" + | ||
| " / / \\ \\\n" + | ||
| " _-| \\___ ___/ /-_\n" + | ||
| " (_ )__\\_)\\(_/__( _)\n" + | ||
| " ))))\\X\\ ((((\n" + | ||
| " \\/ \\/ \n" + |
| byeMessage(); | ||
| lineMessage(); | ||
| } | ||
| public static boolean chatFeature(){ |
There was a problem hiding this comment.
As it's a method, should it be named with a verb?
Created branch-A-Packages for tracker Packages commit in level-5 branch
branch-A-Packages
wenxin-c
left a comment
There was a problem hiding this comment.
Overall good job so far!! Just a few suggestions for you to consider.
| // Continue chatting as long as user doesn't exit | ||
| boolean continueChatting = true; | ||
| while (continueChatting) { | ||
| // Chat feature to handle user input | ||
| continueChatting = chatFeature(); | ||
| } |
There was a problem hiding this comment.
You can consider extracting this part further to match same level of abstraction in this method.
E.g.
readData();
salary = basic * rise + 1000;
tax = (taxable ? salary * 0.07 : 0);
displayResult();
versus
readData();
processData();
displayResult();
| Task.printTasksList(); // Print task list from Task class | ||
| } | ||
| // Handle "mark" command to mark a task as completed | ||
| else if (line.startsWith("mark ")) { |
There was a problem hiding this comment.
Try to avoid magic literals(e.g. numbers, strings), you can consider using named constants instead. It will be useful especially when the particular literals are used multiple times in the code.
E.g.
static final double PI = 3.14236;
static final int MAX_SIZE = 10;
...
return PI;
...
return MAX_SIZE - 1;
There was a problem hiding this comment.
Thanks for the feedback just updated this!
| } | ||
|
|
||
| // Handles user input and executes corresponding actions | ||
| public static boolean chatFeature() { |
There was a problem hiding this comment.
This method is very long, you can consider extracting the details for each case into separate methods. It will be better to keep each method short.
There was a problem hiding this comment.
Thanks for the feedback just updated this! I extracted it into another class to make it easier to read
| } | ||
| // Handle "unmark" command to unmark a task as completed | ||
| else if (line.startsWith("unmark ")) { | ||
| try { |
There was a problem hiding this comment.
Try to keep else if and closing curly bracket on the same line.
E.g.
if (condition) {
statements;
} else if (condition) {
statements;
} else {
statements;
}
There was a problem hiding this comment.
Thanks for the feedback just updated this!
Branch level 6
Implemented Data Writing System
Updating Branch
Implemented DateTime feature and Updated Exception Handling
Branch level 9
Updated JavaDoc Comments
No description provided.