-
Notifications
You must be signed in to change notification settings - Fork 484
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
[billieboy7} iP #519
base: master
Are you sure you want to change the base?
[billieboy7} iP #519
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.
Overall, I found your code easy to read for the most part, except that your code could use a bit of documentation for easier readability. I noted a minor coding violation, but otherwise your code is very well done 👍
src/main/java/Task/Task.java
Outdated
return (isDone ? "X" : " "); // mark done task with X | ||
} | ||
|
||
public void taskDone(boolean status) { |
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.
Minor coding violation! Boolean variables/methods should be named to sound like booleans, and setter methods for boolean variables should be in the following form instead:
public void taskDone(boolean status) { | |
public void setDone(boolean isDone) { |
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.
Noted another minor coding violation, that has to do with one of your methods!
} | ||
|
||
@Override | ||
public boolean end() { |
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.
public boolean end() { | |
public boolean getEnd() { |
src/main/java/Command/Command.java
Outdated
|
||
public void execute(TaskList taskList, UI ui, Storage storage) throws DukeException {} | ||
|
||
public boolean end() { |
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.
public boolean end() { | |
public boolean getEnd() { |
There are no checks currently on how the user input is being handled within the program itself. Using assert statements checks whether the input is being handled properly within the programme and whether each method is working in the way it is intended to. Assert statements are preferred because their less disruptive.
Lack of standardisation of package names threw errors. execute method did not return anything which the gui package could use. Standardised names of the package imports were from. execute method returns string to display on gui.
Insert assert statements
Merge branch 'master' into branch-A-CodeQuality
…ng methods Parser class initially had 2 different operations - 1) To decide the type of command to be returned 2) Check for the validity of the formatting of the command once the type of command to be returned was determined. Abstracting the second operation(validation check) allows the parse method to be simpler and look neater.
Users cant change the description without deleting the task. Introduced a new functionality that allows for the updating of the tasks without deleting the older tasks. Makes it easier, convenient and faster for the updating of task descriptions especially when there are multiple changes.
Duke - Task Tracker
Liberate your mind from the burden of remembering tasks. Duke offers:
speedSPEEDSo what do you have to do :
Awesome Features
If you Java programmer, you can use it to practice Java too. Here's the
main
method: