-
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
[euchangxian] iP #520
base: master
Are you sure you want to change the base?
[euchangxian] iP #520
Conversation
This reduces repeated code.
This is done by adding an overloaded echo() method to maintain the functionality of existing code.
For improved readablity
Update startParse() to first split by spaces to handle commands with multiple words, like mark x or unmark x, where x is the index of the command.
This is to prevent Duke class from getting too bloated.
This is to prevent bloating of Duke class
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 very well written, follows the style guide very well, lgtm, can push to production
* | ||
* @return {@code true} if the file was corrupted, otherwise {@code false}. | ||
*/ | ||
public boolean wasFileCorrupted() { |
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.
Method name should start with a verb, i.e. checkFileCorrupted()
return Optional.of(task); | ||
} | ||
|
||
public List<Task> searchTasks(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 monads
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.
Second this
@@ -0,0 +1,9 @@ | |||
todo master rizz |
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 also wish
This is in preparation to integrate JavaFX with Duke, preventing the Duke class from becoming too bloated.
Refactor existing classes, mainly changing the return type of UiService to return a String which is displayed in the GUI window.
Add a GUI to 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.
LGTM Great job! 😄
throw new InvalidCommandInputException("Provide a keyword for find command! :<"); | ||
} | ||
return new FindTaskCommand(dukeBot, uiService, args.get(0)); | ||
case "todo": |
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.
case "todo": | |
case "todo": | |
// Fallthrough |
The explicit // Fallthrough comment should be included whenever there is a case statement without a break statement.
} | ||
return new FindTaskCommand(dukeBot, uiService, args.get(0)); | ||
case "todo": | ||
case "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.
case "deadline": | |
case "deadline": | |
// Fallthrough |
The explicit // Fallthrough comment should be included whenever there is a case statement without a break statement.
return Optional.of(task); | ||
} | ||
|
||
public List<Task> searchTasks(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.
Second this
|
||
try (FileInputStream fis = new FileInputStream(file); | ||
ObjectInputStream ois = new ObjectInputStream(fis)) { | ||
@SuppressWarnings("unchecked") |
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.
Appropriate use
TaskFactory is responsible for the creation of new Tasks, and in no cases does it return a null task. Since there should only be one way of adding a task to TaskList, by calling TaskList::add, there should be no null tasks added to the TaskList. Neither should there by any null task retrieved by any exposed methods.
This is to better reflect immutability.
Add assert statements to TaskList
Branch-A-Streams
Add java_ci.yml file to ensure code quality
Improve Code Quality
Duke can now parse dates like "monday", "tuesday",..., etc and understands it as the next Monday, Tuesday etc.
Add more special date inputs
If the start date supplied is after the end date, an exception is thrown.
🚀 Changoose 🚀
🎉 This is the latest release of Changoose v0.1! 🎉
🤩 What's New? 🤩
(almost)anything you tell it!🦾 Performance Enhancements 🦾
deadline feed goose \by today
and Changoose will understand you!TODO
Here is the link to the code for Changoose!
Changoose