-
Notifications
You must be signed in to change notification settings - Fork 461
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
[See Toh Jin Wei] iP #16
base: master
Are you sure you want to change the base?
Conversation
Made Task class abstract
Change "true" while loop to check for scanner input instead.
Add error handling and tests for deleting tasks. Improve error handling for mark and unmark operations.
Abstracted out commonly used exceptions.
Navigate with up and down arrow keys
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.
Nice
No issues with naming or layouts
public static final String COMMAND_WORD = "find"; | ||
private static final String TASK_FORMAT = "%d: %s"; | ||
private static final String USER_MESSAGE_FORMAT = "Here are the matching tasks in your list!\n%s"; | ||
private static final DukeException WRONG_FORMAT = | ||
new DukeException("Wrong format for Find!\nShould be 'find <keyword>'."); | ||
private final String keyword; |
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.
Good use of constants
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 taking the time to provide your feedback!
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 u provide additional comments for your variables and code to help me understand better. I like the naming of methods, especially the boolean methods which are really clear what the method is for. The imports are also grouped together for related imports, which is something I like as well. Great use of constants to reduce duplicate code. Very consistent format of code. I like the comprehensive test cases as well. Could not spot any coding standard violations throughout. Well Done!
Code lacks assertions. Add assertions to ensure that certain conditions are met. Let's ensure that tasks are set before executing command.
Some code is too complex. Reduce code complexity, mainly through abstracting long or heavily nested methods. Let's ensure that code quality is kept up.
Find command's execution is confusing. Use streams to improve readability of the code. Let's ensure that streams are used to simplify code.
Add assertions
Improve code quality
There is no way to undo commands. Add undo command to undo commands. Let's support undoing actions.
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 class DeleteCommand extends Command { | ||
/** Command word of the delete command. */ | ||
public static final String COMMAND_WORD = "delete"; | ||
private static final String USER_MESSAGE_FORMAT = "Removing this task!\n %s\nNow you have %d tasks left."; |
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.
Smart use of formatting 👍
* Represents an exception in the Duke application. | ||
*/ | ||
public class DukeException extends Exception { | ||
/** Exception due to an invalid index. */ |
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.
Can extend these into individual exception classes
tasks.addTask(task); | ||
} | ||
} catch (FileNotFoundException e) { | ||
// Should not happen because file is created beforehand. |
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.
Creating file could fail sometimes
- Add welcome message - Add personality - Add bye message with delay before exit - Ignore empty command - Add application title
Duke Application
Duke is great for anyone who keeps a list of tasks! It's
FASTSUPER FAST to runAll you need to do is,
And it is FREE!
Features:
This application is built in Java! Run
Duke
today!SECRET 😉
Please pretend this is a cool secret area!Requirements: