-
Notifications
You must be signed in to change notification settings - Fork 364
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
[chongweiguan] iP #244
base: master
Are you sure you want to change the base?
[chongweiguan] iP #244
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.
Looks good. Just some camelCase violations.
src/main/java/Duke.java
Outdated
LocalDateTime formattedstartTime = LocalDateTime.parse(from, dateTimeFormatter); | ||
LocalDateTime formattedendTime= LocalDateTime.parse(to, dateTimeFormatter); | ||
Event event = new Event(command.substring(0,fromStart-7), formattedstartTime, formattedendTime); |
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.
Hi, I'm your commenter and I would like to confirm whether the camelCase for these variable declarations has been fixed? Otherwise, great job on the clean code!
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.
Code looks good overall bro 👍 Keep it up!
src/main/java/duke/Duke.java
Outdated
import java.io.FileReader; | ||
import java.io.FileWriter; | ||
import java.io.IOException; | ||
import java.util.*; |
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 might want to list out the specific imports instead of using a wildcard import
src/main/java/duke/UI.java
Outdated
|
||
String[] taskDescription = list.getTask(i).getDescription().split(" "); | ||
for (String word : taskDescription) { | ||
if(keyword.equals(word)) { |
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 standard violation but there should be a whitespace after 'if'
This reverts commit 8aaa678.
Merge A-Assertion include assert statements in java files when required
A-CodeQuality
Code quality fixes on all java files, made method length shorter and added Javadoc comments Good software engineering practices Made method length shorter by abstracting out the function into separate methods, and added Javadoc comments for methods without JavaDoc comments Abstracting out the functions into separate methods is preferable that helps with reading
Documentation for class fields added for better readability
Added javaDoc comments
A-CodeQuality
Added more JavaDoc for documentation and good software engineering practices
Add a feature that allows a user to add a task that needs to be done within a certain period
Trim out trailing white spaces
A-CodeQuality
Add JavaDoc description to ensure better readability and better documentation
Add JavaDoc for setDuke method in MainWindow.java It allows for better readability and documentation is good software engineering practice
DukeBot
DukePro frees your mind of having to remember things you need to do. It's,
FASTSUPER FAST to useWhere do I begin?
All you need to do is,
And it is FREE
Features