-
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
[Tan Hong Liang] iP #501
base: master
Are you sure you want to change the base?
[Tan Hong Liang] iP #501
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.
Overall, the code was nicely designed, the code style follows coding standards most of the time, other than a few white space and JavaDoc issues. Looks good!
src/main/java/Duke.java
Outdated
@@ -10,13 +10,15 @@ public class Duke { | |||
private static final String COMMAND_LIST = "list"; | |||
private static final String COMMAND_BYE = "bye"; | |||
private static final String COMMAND_TODO = "todo"; | |||
private static final String COMMAND_DEADLINE = "deadline"; | |||
private static final String COMMAND_EVENT = "event"; | |||
private static final String COMMAND_MARK = "mark"; | |||
private static final String COMMAND_UNMARK = "unmark"; | |||
|
|||
public static void main(String[] 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 love the design of code, but it would be great if the main "command execution" part could be extracted out to another class (which is required in Week 3 ip task [A-MoreOOP], so it might be easier if we change the code earlier)
src/main/java/Duke.java
Outdated
@@ -77,8 +107,8 @@ public static String outputNumOfTasks() { | |||
return String.format("Now you have %d tasks in the list \n", tasks.size()); | |||
} | |||
|
|||
public static String chat(String message) { | |||
return UI_LINE_SPACING + message + UI_LINE_SPACING; | |||
public static void chat(String message) { |
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.
It would be great if JavaDoc could be added (but I highly understand that you might want to add comments once for all when the variables/func names are confirmed.
src/main/java/Event.java
Outdated
@@ -0,0 +1,14 @@ | |||
public class Event extends 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.
I think you might need a white space before the curly brace according to the code standard.
src/main/java/Task.java
Outdated
@@ -0,0 +1,36 @@ | |||
public class Task { | |||
protected String description; | |||
protected boolean isDone; |
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.
Seems that there is an extra white space followed by "protected" according to coding standard.
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.
Yes this violates the coding standard
src/main/java/Deadline.java
Outdated
@@ -0,0 +1,14 @@ | |||
public class Deadline extends 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.
Similar to Event class, I think you might need a white space before the curly brace according to the code standard.
src/main/java/Duke.java
Outdated
@@ -1,6 +1,5 @@ | |||
import java.util.Scanner; | |||
import java.util.ArrayList; | |||
|
|||
public class Duke { | |||
private static ArrayList<Task> 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.
can consider making this a private static final variable
src/main/java/Duke.java
Outdated
public static void error(String message) { | ||
System.out.println(UI_LINE_SPACING + ":( OOPS: " + message + "\n" + UI_LINE_SPACING); | ||
} | ||
|
||
public static void end() { | ||
System.out.println(UI_LINE_SPACING + "Bye! Hope to see you again!" + UI_LINE_SPACING); | ||
isEnd = true; |
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 code is clear and the logic flows and should have no problem completing the IP
src/main/java/Task.java
Outdated
@@ -0,0 +1,36 @@ | |||
public class Task { | |||
protected String description; | |||
protected boolean isDone; |
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.
Yes this violates the coding standard
Level-7 requirements done, saving and loading finished, merged with master
Level-8 done, date time changed to LocalDate, merged with master
…starting to make the individual classes work
… from, start incorporating in some classes.
…ing the Command classes which will return tasks, also fixed all errors such that chatbot works now. Package splitting finished as well.
…ursory check for coding standards will continue to check
Level-Javadoc: Added documentation to all classes except tasks classes, documentation is surface level for now will add more
…ress to save to and load from
… code quality as I believe it also falls under this category
Level-A-CodeQuality PR merge commit
…s can only do so by exiting the programme, which would save it in the log This makes it very difficult for them to have multiple save states of their tasks, which may be necessary for users who want to use the scheduler for repetitive tasks, or for trying out different planned schedules and seeing which fit As such, I have decided to add the archive functionality which lets users save their current configuration within a archive file. The archive is a collection of text files that saves logs of the tasklist and is stored in the Resources/archive directory I decided to use archiving as it makes it more customisable and flexible for the users new commands from the archive addition: - archive [archive name] - loadA [archive name] - listA
Currently there is no user guide on the github New users would not have a user guide to refer to when just starting to use the program Thus I added a user guide that would be able to tell users about the features and usage of the program This user guide will be the first thing shown when a user enters the github repo, thus allowing them easy access to it
Added in the clear command which was just updated
Duke iP
This Duke chatbot is an interactive task scheduler that allows users to:
All you need to do is to:
and it's FREE!
Features:
Here's a code snippet to get you excited!