-
Notifications
You must be signed in to change notification settings - Fork 435
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
[yyccbb] iP #454
base: master
Are you sure you want to change the base?
[yyccbb] iP #454
Conversation
Let's tweak the docs/README.md (which is used as the user guide) to fit Duke better. Specifically, 1. mention product name in the title 2. mention adding a product screenshot and a product intro 3. tweak the flow to describe feature-by-feature
DataHandler class does not support local data persistence. The list of tasks created gets wiped away after one Nihao instance is closed. Adding support for local data persistence allows users to save the tasks so that they still can access them after closing the app. Let's use JSON serialization and deserialisation functionalities to load and save the list of tasks to a local JSON file. JSON is used because it is possibly easier for integration with databases.
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. Good code quality following naming standards.
|
||
public class DataHandler { | ||
private static final String fileNamePath = "data/PersistentData.json"; | ||
private static ArrayList<Task> tasks = new ArrayList<>(); |
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 naming separating single and multi-valued variables! 💯
public static void printWithDivider(String msg) { | ||
System.out.println(msg); | ||
System.out.println(DIVIDER); | ||
} | ||
public static void printNumberedDivider(ArrayList<Task> msgs) { |
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 and clear naming of methods! 💯
|
||
@Override | ||
public Task read(JsonReader jsonReader) throws IOException { | ||
Task ret = null; |
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.
Would there a better and clearer variable name for the task than ret? 🤔
private int countByFlag(String[] parsedInput) { | ||
int counter = 0; | ||
for (int i = 0; i < parsedInput.length; i++) { | ||
if (parsedInput[i].equals("/by")) { | ||
counter++; | ||
} | ||
} | ||
return counter; | ||
} | ||
|
||
private int countFromFlag(String[] parsedInput) { | ||
int counter = 0; | ||
for (int i = 0; i < parsedInput.length; i++) { | ||
if (parsedInput[i].equals("/from")) { | ||
counter++; | ||
} | ||
} | ||
return counter; | ||
} | ||
|
||
private int countToFlag(String[] parsedInput) { | ||
int counter = 0; | ||
for (int i = 0; i < parsedInput.length; i++) { | ||
if (parsedInput[i].equals("/to")) { | ||
counter++; | ||
} | ||
} | ||
return counter; |
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 like how clean this is! Would consider applying to my code too! 💯
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 the feedback!
Task classes store timestamps as String. Storing timestamps as LocalDateTime objects is preferred for formatting purposes and to prevent users from writing irrelevant strings as timestamps. Let's, * create DateTimeHandler singleton class to handle conversion between String and LocalDateTime ojbects * create IncorrectDateTimeFormatException class which produces exceptions to be thrown when a String that does not follow a given format attempts to get converted * update Task classes to store LocalDateTime objects instead of Strings * update Task classes' toString() methods to follow the specified format * update TaskTypeAdapter to support LocalDateTime objects
"bye" command is handled directly by Nihao class. This does not conform to the rest of the codebase where commands are handled by InputHandler. Let's create an ExitAction class to handle "bye" command.
# Conflicts: # docs/README.md
Action and Task classes do not have custom equals() methods. The equals() method is needed for assertEquals() to assert equality between action and task objects based on the needs of this application. Let's create equals() methods for all Action and Task classes and create InputHandlerTest class to test for InputHandler class.
Nihao does not have a GUI. A GUI is more user-friendly. Let's * use JavaFX to create a GUI * add start() method to Nihao as an entry point * create DialogueBox class to generate reusable dialogue boxes * update PrintHandler methods so that they return the content to be printed as String as well * update Task.execute() so that they return the content to be printed as String, which is then packed up as dialogue boxes
Assertions are not used anywhere in the codebase. They can be helpful in verifying the state of the program at strategic points. Let's, * use assertions in the execute() methods of action classes to confirm that key fields have legal values when execute() is invocated * use assertions in the getResponse() method of Nihao to confirm that the text input is not null
Merge CodeQuality with master
Revert "Merge CodeQuality with master"
Merge Assertions with master
Merge CodeQuality with master
Merge CodeQuality with master
Nihao
Nihao frees your mind of having to remember things you need to do. It's,
FASTSUPER FAST to useAll you need to do is,
And it is FREE!
Features:
If you are a Java programmer, you can use it to practice Java too. Here's the
main
method: