-
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
[Tran Gia Phong] ip #189
base: master
Are you sure you want to change the base?
[Tran Gia Phong] ip #189
Conversation
Implement a skeletal version of Duke that starts by greeting the user, simply echos commands entered by the user, and exits when the user types bye.
Add the ability to store whatever text entered by the user and display them back to the user when requested.
Add the ability to mark tasks as done.
Add support for tracking three types of tasks: ToDos, Deadlines and Events
Teach Duke to deal with errors such as incorrect inputs entered by the user.
Add support for deleting tasks from the list.
…en calling DELETE and DONE command with out-of-range parameters
…of-range parameters
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 think generally ok just need to add in some Javadocs and new lines and rearrange import statements. Nice Job!
+ "tasks list: " + taskList; | ||
} | ||
} | ||
|
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.
Should there be a JavaDoc Comment here?
src/main/java/duke/Duke.java
Outdated
while (!input.equals("bye")); | ||
Ui.close(); | ||
} | ||
|
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.
Should there be a JavaDoc Comment here?
src/main/java/duke/Launcher.java
Outdated
public static void main(String[] args) { | ||
Application.launch(Main.class, args); | ||
} | ||
} |
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 think you might want to add newline at end of file
src/main/java/duke/Main.java
Outdated
e.printStackTrace(); | ||
} | ||
} | ||
} |
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.
Perhaps also adding a newline at the end of file would be good?
src/main/java/duke/MainWindow.java
Outdated
); | ||
userInput.clear(); | ||
} | ||
} |
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.
Perhaps also adding a newline at the end of file would be good?
The newline issue seems to be prevalent among other files, so perhaps it might be good to go about adding them to all the files.
@@ -0,0 +1,28 @@ | |||
package misc; | |||
|
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.
Should the import statements be arranged this way?
|
||
import static org.junit.jupiter.api.Assertions.assertEquals; | ||
|
||
public class ParserTest { |
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.
Should there be a Javadoc comment here?
List<String> expected = new ArrayList<>(Arrays.asList("deadline", "read book", "21/12/2012 1900", "0")); | ||
assertEquals(expected, actual); | ||
} | ||
|
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.
Should there be a Javadoc comment here?
@@ -0,0 +1,19 @@ | |||
package 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.
Should the import statements be arranged this way?
import static org.junit.jupiter.api.Assertions.assertEquals; | ||
|
||
public class DeadlineTest { | ||
|
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.
Should there be a Javadoc comment here?
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 clean code! I was really impressed and learned a lot. I couldn't really find anything to correct.
src/main/java/duke/Duke.java
Outdated
} | ||
|
||
/** | ||
* Run the program. |
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.
Should this be Runs instead of Run?
* Create a task with a title and its optional datetime | ||
* | ||
* @param description the title of the task | ||
* @param time the time of the 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.
Did you accidentally leave a space here between "time" and "the"?
Refactor code
Add assertions
No description provided.