-
Notifications
You must be signed in to change notification settings - Fork 434
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Darkarche3]iP #289
base: master
Are you sure you want to change the base?
[Darkarche3]iP #289
Conversation
Let's tweak the docs/README.md (which is used as the user guide) to fit Duke better. Specifically, 1. mention product name in the title 2. mention adding a product screenshot and a product intro 3. tweak the flow to describe feature-by-feature
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, just a few coding standard violations.
src/main/java/Duke.java
Outdated
@@ -1,10 +1,100 @@ | |||
import java.util.ArrayList; | |||
import java.util.Scanner; | |||
public class Duke { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe can change the class name to your bot's name?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks. I will do so!
src/main/java/Duke.java
Outdated
while(true){ | ||
if(sc.hasNextLine()) { | ||
// checks if there is another instruction | ||
instruction = sc.nextLine(); | ||
}else{ | ||
// ends conversation | ||
sc.close(); | ||
break; | ||
} | ||
if(instruction.equals("bye")){ // command to end chat with chatbot | ||
System.out.println("Bye. Hope to see you again soon!"); | ||
break; | ||
}else if(instruction.equals("list")){ | ||
// lists out all tasks | ||
for(int i=0;i<count;i++){ | ||
System.out.println(i + 1 + "." + storage.get(i).toString()); | ||
} | ||
System.out.println(); | ||
}else if(instruction.contains("unmark")){ | ||
// marks a specific task as undone | ||
String[] arr = instruction.split(" "); | ||
int num = Integer.parseInt(arr[1]) - 1; // task to be unmarked | ||
storage.get(num).taskUndone(); //marks a task as done | ||
System.out.println("OK, I've marked this task as not done yet:\n " + | ||
storage.get(num).toString()); | ||
}else if(instruction.contains("mark")){ | ||
// marks a task as done | ||
String[] arr = instruction.split(" "); // splits the task into individual word | ||
int num = Integer.parseInt(arr[1]) - 1; // task to be unmarked | ||
storage.get(num).taskDone(); // marks a task as done | ||
System.out.println("Nice! I've marked this task as done:\n " + | ||
storage.get(num).toString()); | ||
}else if(instruction.contains("deadline")){ | ||
if(instruction.equals("deadline")){ | ||
// throws error for incomplete instruction | ||
DukeException error = new DukeException(instruction); | ||
System.out.println(error.toString()); | ||
}else { | ||
String[] arr = instruction.split(" /by "); // splits the task into description and deadline | ||
storage.add(new Deadline(arr[0].substring(9), arr[1])); | ||
count++; // keep track of number of tasks | ||
System.out.println("Got it. I've added this task:\n" + storage.get(count - 1).toString() | ||
+ "\n" + "Now you have " + count + " tasks in the list" + ".\n"); | ||
} | ||
}else if(instruction.contains("todo")){ | ||
if(instruction.equals("todo")){ | ||
// throws error for incomplete instruction | ||
DukeException error = new DukeException(instruction); | ||
System.out.println(error.toString()); | ||
}else { | ||
storage.add(new Todo(instruction.substring(5))); | ||
count++; //keep track of number of tasks | ||
System.out.println("Got it. I've added this task:\n" + storage.get(count - 1).toString() | ||
+ "\n" + "Now you have " + count + " tasks in the list" + ".\n"); | ||
} | ||
}else if(instruction.contains("event")){ | ||
if(instruction.equals("event")){ | ||
// throw error for incomplete instruction | ||
DukeException error = new DukeException(instruction); | ||
System.out.println(error.toString()); | ||
}else { | ||
String[] arr = instruction.split(" /from "); // split task into description and deadline | ||
String[] arr1 = arr[1].split(" /to "); // split deadline into from and to | ||
storage.add(new Event(arr[0].substring(6), arr1[0], arr1[1])); | ||
count++; // keeps track of tasks | ||
System.out.println("Got it. I've added this task:\n" + storage.get(count - 1).toString() | ||
+ "\n" + "Now you have " + count + " tasks in the list" + ".\n"); | ||
} | ||
}else if(instruction.contains("delete")){ | ||
String[] arr = instruction.split(" "); // split instruction into delete and index of item to be deleted | ||
Task removal = storage.get(Integer.parseInt(arr[1]) - 1); // task to be removed | ||
storage.remove(Integer.parseInt(arr[1]) - 1); | ||
count -=1; | ||
System.out.println("Noted. I've removed this task:\n" + removal.toString() + "\n" + "Now you have " | ||
+ count + " tasks in the list.\n"); | ||
|
||
}else{ | ||
/* | ||
any instruction that is not clear that has not | ||
been covered | ||
*/ | ||
DukeException error = new DukeException(instruction); | ||
System.out.println(error.toString()); | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe can indent your parenthesis based on the coding standard? Eg. while () and if ()
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for pointing it out!
src/main/java/DukeException.java
Outdated
if(this.description.equals("todo") || this.description.equals("deadline")||this.description.equals("event")){ | ||
return "Your input is incomplete. Please add more details for " + this.description + "."; | ||
}else{ | ||
return "I do not understand your input."; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe can indent your parenthesis based on the coding standard? Eg. while () and if ()
src/main/java/Deadline.java
Outdated
|
||
@Override | ||
public String toString() { | ||
return "[D]" + super.toString() + " (by: " + by + ")"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like how you called the super class method
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Just a few nits to fix.
src/main/java/Duke.java
Outdated
System.out.println("Hello! I'm Area.\n" + | ||
"What can I do for you?\n"); | ||
|
||
boolean chatting = true; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps you could change the variable name to sound like a boolean to align better with naming practices
boolean chatting = true; | |
boolean isChatting = true; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks
src/main/java/Duke.java
Outdated
System.out.println("Got it. I've added this task:\n" + storage.get(count - 1).toString() | ||
+ "\n" + "Now you have " + count + " tasks in the list" + ".\n"); | ||
} else if (command.equals("deadline")) { | ||
String[] arr = description.split(" /by "); // splits the task into description and deadline |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps a more descriptive variable could be used?
src/main/java/Duke.java
Outdated
public class Duke { | ||
static ArrayList<Task> storage; | ||
static ArrayList<String> instructions = new ArrayList<String>(); | ||
static int count = 0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe a more explanatory name for count could be used to explain what this variable is counting?
static int count = 0; | |
static int taskCount = 0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks!
src/main/java/Task.java
Outdated
/** | ||
* sets work as done | ||
*/ | ||
public void taskDone() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe this could be worded more as a verb to indicate what this method is doing precisely?
public void taskDone() { | |
public void markTaskAsDone() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
will do so. Thank you for the help!
Following Git conventions can help make it easier to understand and follow the codebase. Any changes made will also be easy to understand.
The find method will help make it possible to quickly search for certain task based on keywords.
Updated comments to follow Coding standards to ensure it is easy to read for others.
src/main/java/Duke.java
Outdated
storage[count] = new Event(arr[0].substring(6), arr1[0], arr1[1]); | ||
count++; | ||
System.out.println("Got it. I've added this task:\n" + storage[count - 1].toString() | ||
String[] arr = instruction.split(" /from "); // split task into description and deadline |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some variable names are not descriptive (like arr and arr1). Maybe more descriptive names are better. (This is hypocritical as I do the same)
src/main/java/Duke.java
Outdated
while(true){ | ||
if(sc.hasNextLine()) { | ||
// checks if there is another instruction | ||
instruction = sc.nextLine(); | ||
}else{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Small styling violation as, if I am not wrong, there should be a space between the brackets and the condition.
}else{ | |
} else { |
Bug Fix: Add assert to minimise incorrect input. Certain classes use inputs to create Task object or child of Task object. Exceptions are used to tackle errors in input by user. However, assertions help internal logic check to ensure code functions correctly. For example, assert is used to check that deadline set is in the future and not in the past. Let's add assert statement to ensure that code logic. This ensure the the code gives correct output to user and there is no logic error when using chatbot.
improve security: Add assert to check for incorrect input
Improve Code Quality There was some code that did not meet coding standards. Modify certains part of the code to include proper spacing. Modify header comments to give a more accurate description of method This will help improve readability and quality of code.
Error while merging branches A-CodeQuality and master. This is resolved to allow the 2 branches to merge. Add assert statement to check count is greater than or equal to 0. This prevents count from negative.
Improved Code Quality
Task class: add priority As part of Week 5, need to allow chatbot to assign priority to task based on user. Add methods to allow priority to be changed based on user input Initialise priority as 0 when new Task is created. This allows easy change of priority for a particular task.
BCD Extension
Forgot to create branch-Level-10. Delete redundant method.
Added UI.png as part of week 6 task requirements Add Ui.png to docs folder.
Readme is incomplete. Updating Readme allows for user to understand how to use the chatbot. Update Readme by adding features of chatbot such as the commands. This is to make it easy and direct for users to understand what commands the chatbot can follow.
Junit Tests did not include Priority Changed tests to include Priority attribute in Task creation Update output in assertEquals in DeadlineTest
Fixed certain files to get the necessary error message or output Update the files to reflect these changes.
Error message was created when launching jar file Update createNewFile method to resolve error
Changed parsePriority method to return error message based on incorrect input Update parsePriority method
Removed AreaExeception.java class as it was redundant.
Updated javadocs so that all methods and classes had their respective javadocs comments.
Updated Javadocs Made some minor edits in code.
AreaPro
AreaPro frees your mind of having to remember things you need to do. It's,
+easy to learn
FastSUPER FAST to useAll you need is to
It is also FREE!
Features include:
If you are a Java programmer, you can use it to practice Java too. Here's the main method: