-
Notifications
You must be signed in to change notification settings - Fork 437
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
[Tan Kang Liang] iP #308
base: master
Are you sure you want to change the base?
[Tan Kang Liang] iP #308
Conversation
* @param input Input from user. | ||
*/ | ||
private void listByDueDate(String input) { | ||
Optional<LocalDate> optionalDate; |
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 use of Optional!
src/main/java/duck/Duck.java
Outdated
|
||
/** | ||
* Receives input continuously until "bye" command is given. | ||
* Main loop of the bot. |
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.
JavaDocs can be updated to account for the return type of String[] this time
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 pointing that out!
/** | ||
* LocalStorage makes use of Serializable objects to write to a local file. | ||
*/ | ||
public class LocalStorage implements Storage { |
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.
Really neat exemplification of abstraction with interfaces for Storage implemented by main class LocalStorage!
private List<Task> tasks; | ||
|
||
public TaskList() { | ||
this.tasks = new ArrayList<Task>(); |
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. can be removed for better readability
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! I'll remove all the uses that don't disambiguate anything.
*/ | ||
public String[] getStatusesByDate(Optional<LocalDate> optionalDate) { | ||
|
||
return this.tasks.stream() |
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.
Awesome use of streams & FP to filter out the tasks with dates!
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.
Your code was really cool and I had a great revision on my forgotten knowledge of Java streams & FP. Just some really minor issues with the code readability but overall it's awesome!
Add assertion for date in TaskWithDate
Fix code quality
Add lambda expressions
Update streams to use method call
No description provided.