-
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
[Cocoanautz] ip #219
base: master
Are you sure you want to change the base?
[Cocoanautz] ip #219
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
# Conflicts: # src/main/java/Duke.java
Date datatype in Deadline and Event objects have been updated from String to LocalDate to support more date related functions.
Moved the save and load functions out of the main method to decouple storage functionality from main logic. Storage is now accessed with filePath and Arraylist.
Refactored UI, Storage, Parsing and List functionality into respective packages and classes.
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. There are only some minor code quality issues that you can easily fix and missing javadocs for some of your classes and methods. Well Done!
this.isDone = false; | ||
} | ||
public String getStatusIcon() { | ||
return (isDone ? "X" : " "); // mark done task with X |
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.
Do not repeat in comments information that is already obvious from the code
src/main/java/Drew.java
Outdated
throw new InsufficientArgumentsException("'Mark index' cannot be empty"); | ||
} | ||
break; | ||
//valid integer will be checked later |
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.
Comments should target other programmers reading the code and not only make sense to the person who wrote it
src/main/java/Drew.java
Outdated
* @param input String containing full user input. | ||
* @return Command specified by user input. | ||
*/ | ||
public static Command checkCommandIdentity(String input) throws UnknownCommandException, |
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.
Avoid complicated expressions and definitions. Your method checkCommandIdentity
gets the userCommand by checking for string equivalence, then uses the userCommand to perform a conditional switch case check. This can be combined into 1 and reduce redundant checkings and simplify your method to make it more easily readble
src/main/java/Drew.java
Outdated
} | ||
} | ||
|
||
public static String executeCommand(ArrayList<Task> ls, String 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.
Avoid Long Methods, you can consider calling another method to process your different userCommands
Unit tests currently cover Parser.checkCommandIdentity() and Storage.parseSave(). For checkCommandIdentity(), it tests whether the parser recognizes the identity of valid commands. For parseSave(), it tests for corruption in status symbol, argument count and date format issues.
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.
Overall, the code followed the coding standard well with appropriate naming and statements. JavaDocs were missing for only 1-2 methods.
src/main/java/drew/ui/Parser.java
Outdated
|
||
int inputLength = input.length(); | ||
Command userCommand; | ||
if (inputLength == 3 && input.substring(0, 3).equalsIgnoreCase("bye")) { |
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 could name input.substring(start, end) as "String command".
src/main/java/drew/ui/Ui.java
Outdated
System.out.println(DELIMITER); | ||
System.out.println("Hello! I'm drew.Drew"); | ||
System.out.println("What can I do for you?"); | ||
System.out.println(DELIMITER); |
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 could combine them into a constant String GREETING
protected String description; | ||
protected boolean isDone; | ||
|
||
public Task(String description) { |
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.
Could add JavaDocs, minor issue
return "[" + getStatusIcon() + "] " + description; | ||
} | ||
|
||
public abstract String toSaveFormatString(); |
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 could add a description.
Separated command execution logic from parser. Parser only checks for command identity now.
# Conflicts: # src/main/java/drew/Drew.java # src/main/java/drew/ui/Parser.java
Most of the classes and methods minor checkstyle issues related to: 1. Class JavaDocs 2. Import statement ordering 3. Coding standards. Fixing these issues will increase code readability, as well as ensure that styles are consistent across all files.
Asserts were added into some commands to ensure that variables are within the expected values during runtime. These changes would increase the reliability of the program.
Add asserts in commands
The following extension have been added: 1. B-Reminders 2. C-DetectDuplicates Reminders: The chatbot would remind the user of any upcoming deadlines whenever it starts. DetectDuplicates: The chatbot would prevent the user from creating tasks that already exist.
Previously, Dialog boxes would terminate after 4 lines. This commits removed max dialog box size.
Drew, The chat-bot of all time.
Do you want to be:
If you answered YES to all the above, then Drew is The buddy for you!
All you need to do to achieve greatness is:
Features include:
Look at how cool this response in the
executeCommand
method is: