-
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
[Eugene Ong Wei Xiang] iP #29
base: master
Are you sure you want to change the base?
Conversation
The program starts with a fresh empty list on startup. Users will be able to use it as a long-term task manager if they are able to save their tasks. Implemented a Storage class that handles reading from and writing to a File stored in /data/.
Time and Date for Tasks are stored as Strings. This imposes limitations on possible future features like filtering Tasks by date. TimeStamp class implemented to store date and time information.
Merge Conflict resolved in Duke.java
Alot of code resides in Duke.java Related functions should be bundled into new classes so future changes will be easier to implement. Extracted Ui, Parser, TaskList classes from Duke.java Now, Duke.java does not hold any complicated code.
Name alternated between Duke and Alfred. Confusing for users. Name has been standardized to Ren. ren package and ren.task package added.
Added JUnit Tests for 6 out of 11 classes. Classes without JUnit Tests: * Ren * Ui * TaskList * Storage * Tasks
Add JavaDoc for all non-private classes/methods Add JavaDoc for all non-trivial private methods.
Fixed test codes to comply with Java Standards.
Users are unable to easily find a task from a dense list of tasks. Users are discouraged from using Ren to track many tasks. Users can now search for tasks with keywords.
1 error caught in 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.
Heyo not sure if I'm doing this correctly but lemme know if you have any questions! Not much problems with formatting that I can find haha
src/main/java/DukeException.java
Outdated
|
||
@Override | ||
public String toString() { | ||
return " ☹ Apologies! "+ super.getMessage() + "\n"; |
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.
In general, I really liked how you separated your toStrings via line breaks to improve readability!
Small comment here in line 8 that there should be a spacing after the string, so
" ☹ Apologies! " +
src/main/java/Duke.java
Outdated
} else { | ||
throw new DukeException("Please indicate a task no. between 1 to " + pointer + "."); | ||
} | ||
} | ||
|
||
private static void updateTask(TaskFunc func, int taskNum) throws DukeException { | ||
private void updateTask(TaskAction func, int taskNum) throws DukeException { | ||
if (taskNum <= pointer && taskNum > 0) { | ||
switch (func) { | ||
case MARK: |
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 good use of enums! Properly indented as well might just steal it heh
src/main/java/Storage.java
Outdated
for (int i = 0; i < this.counter; i++) { | ||
String[] data = dataArray.get(i).split("\\|"); | ||
Task newTask = null; | ||
switch (data[0]) { |
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 data[0] could be given a more meaningful name, perhaps something like:
String dataHeader = data[0];
switch (dataHeader) {
My loading is very similar to yours and I just realized it might be a bit confusing for people who didn't split their save data by lines. :)
@@ -22,6 +22,7 @@ private TimeStamp(LocalDateTime timestamp) { | |||
* @throws RenException If dateTime is not formatted correctly. | |||
*/ | |||
public static TimeStamp of(String dateTime) throws RenException { |
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 is completely up to you but I felt the method name for this factory method is a little short? Perhaps something like
public static TimeStamp **generateTimestamp**(String dateTime) throws RenException
could be something a bit more descriptive? I do understand that "of" is used for the grammar to flow in another context, so feel free to ignore this!
src/main/java/Deadline.java
Outdated
@@ -1,6 +1,15 @@ | |||
/** |
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.
Sorry I just saw you had Checkstyle added so ignore all the previous mentions of spacing issues, they should be resolved by here. Regardless formatting wise I mean there aren't any styling violations apart from some small naming potential improvements! Really good job man :)
Ren runs on a text ui. This is unlikely to be pleasing to users. Let's upgrade the ui to a GUI that resembles a chat app. This would make the program more pleasing to use.
Currently, important assumptions are checked using if statements or not checked at all. This is dangerous as it is not guaranteed that the assumptions will hold. Additionally, there is a risk that some assumptions might be forgotten about. Now, all important assumptions are checked using Assertions. Assertions allow assumptions to be clearly marked out from other code.
Currently, the code does not strictly follow the newly introduced Code Quality guidelines. This indicates areas of improvements in the code readability. Now, after combing through the code with the new guidelines, several changes have been made to improve code readability.
Improve Code Quality
Add and enable Assertions
Currently, the list of tasks is not sorted and left in the order the user added them. This hampers the user's ability to organise their tasks by certain characteristics. Now that the sort feature is implemented, the user can sort tasks by: * their type (Todo > Deadline > Event) * their status (Unmarked Tasks > Marked Tasks) * their description (sorted lexicographically while ignoring case) * their date (sorted chronologically while Todo's are sorted last)
Product Screenshot included. User Guide v0.1 included.
Ren
Ren 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:
Here's a sneak peek at the
main
method of the next iteration.