-
Notifications
You must be signed in to change notification settings - Fork 462
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
[Samuel Pang Shao Herng] ip #517
base: master
Are you sure you want to change the base?
Conversation
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.
In general, I think you could try to line wrap some of the long lines as there are many instances of long lines in the code.
src/main/java/Deadline.java
Outdated
@@ -0,0 +1,14 @@ | |||
public class Deadline extends Task { | |||
|
|||
protected String 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.
This variable is only used within the class so it could use the private access modifier instead
src/main/java/Duke.java
Outdated
} | ||
|
||
public String addTaskMessage(String taskString) { | ||
return horizontalBorder + "Got it. I've added this task:\n" + taskString + "\n" + "Now you have " + this.tasklist.getCount() + " tasks in the list.\n" + horizontalBorder; |
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.
Line length for this line is pretty long so you could look into using line wrapping such as splitting at the addition operators.
src/main/java/Duke.java
Outdated
System.out.println(welcomeMessage()); | ||
Scanner scan = new Scanner(System.in); | ||
String s = scan.nextLine(); | ||
boolean exitNow = false; |
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.
You could consider using a variable name which sounds more like a Boolean such as isExit.
src/main/java/Duke.java
Outdated
try { | ||
String[] commandList = s.strip().split(" "); | ||
String command = commandList[0].toLowerCase(); | ||
if (command.equals("bye") && commandList.length == 1) { |
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.
You can consider using switch statements here.
src/main/java/Duke.java
Outdated
|
||
private Tasklist tasklist; | ||
|
||
private 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.
It does not appear that this constructor is invoked so you could look into omitting this constructor.
Added Gradle support.
Added DeadlineTest and ParserTest classes to conduct testing on the methods of Deadline.java and Parser.java respectively
Added Javadocs for classes in the duke.command package, duke.task package, duke.ui package
Modified the files to ensure switch case statements now have no indentation and Java's conventions with regards to wrapping, newline, whitespace are adhered to.
Added the find feature which allows user to find matching tasks using a keyword
Updated duke with GUI using JavaFX
Assertion statements are able to complement exception handling in handling errors. It is especially useful in indicating mistakes in the code returned Let's, * Write some assertion statements across multiple files to verify the correctness of our code
Certain repetitive tasks existed in the code base, and thus it should abstracted into a method for improvements in code quality and readability. Let's, * Abstract out certain repetitive methods in Storage.java to improve readability and shorten method length * Declare some variables in order to help improve readability and reader understanding
Add assertion statements
There was a merge conflict encountered when trying to merge master and branch-A-Assertions. Lets, - Resolve the merge conflict so that any discrepancies can be successfully resolved
There was a merge conflict encountered when trying to merge master and branch-A-CodeQuality. Lets, * Resolve the merge conflict so that any discrepancies can be successfully resolved
Prior to implementation of extension, users may not have much idea on how to use Duke properly. Let's, * Implement a help feature so that users can view all the commands once after they input "help" in the text field * Implement a advanced help feature so that users can view advanced commands to better utilise Duke
There exists a help and an advanced help message to guide the user on what commands can be utilised. This means that the invalid user input message which guides the user on Duke commands is not necessary. Let's, * change the content in the invalid user input message to be more meaningful and more concise, providing a shorter and more readable instruction for the user
Duke application lacks a user guide to guide the user how to use Duke. Let's, * Create a user guide to give brief instructions to the user to utilise Duke
Some comments were added to indicate some appropriation of code from multiple sources.
Ammended some formatting errors from README.md
Rectified spelling errors of README.md
Proper line spacing was implemented to further categorise portions of the README.md
Some errors in line spacing occured, thus README.md was modified to resolve such errors
Duke
DukePro frees your mind of having to remember things you need to do. It's,
FASTSUPER FAST to useAll you need to do is,
Features :
If you Java programmer, you can use it to practice Java too. Here's the
main
method: