[Lim Yong En Dominic] iP #462
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
…ke to required IP level 0 text.
…ies based on level-4
…ing for tasks concerning time like deadline and event.
…Zoe will read saved tasks from previous sessions and she will save new tasks from the current session.
# Conflicts: # data/SavedTasks.txt # src/main/java/Deadline.java
…nerate zoe jar files.
# Conflicts: # src/main/java/zoe/Parser.java # src/main/java/zoe/Ui.java
| @@ -0,0 +1,52 @@ | |||
| package zoe; | |||
|
|
|||
| import java.io.*; | |||
There was a problem hiding this comment.
Should you list your imported classes explicitly to follow the Java Coding Standard?
| package zoe; | ||
| public class Zoe{ | ||
| private Parser parser; | ||
| private TaskList tasks; |
There was a problem hiding this comment.
I like that you have distinguish clearly between single-valued and multi-valued variables, nice!
| * If invalid command like empty todo is given, calls the respective responses | ||
| * @param command | ||
| */ | ||
| public String carryOutShortCommand(String command) { |
There was a problem hiding this comment.
It is good that your method name explains the entity to the reader accurately and at a sufficient level of detail, nice!
| * Saves data from the current zoe instance to a path when zoe closes | ||
| * @param path | ||
| */ | ||
| public void saveTo(String path) { |
There was a problem hiding this comment.
It is good that your methods and classes follow the Java Code Quality of using nouns for classes/variables and verbs for methods/functions, nice!
src/main/java/zoe/TaskList.java
Outdated
| } | ||
|
|
||
| /** | ||
| * Checks if a task exists in the list given an index when isValid(index) is called. |
There was a problem hiding this comment.
I noticed that there is no line spacing in between the description and parameter sections for your Javadocs comments, should you consider following the Java Coding Standard?
One of the comment does not have a method associated with it, should you consider removing it?
|
|
||
| protected String date; | ||
| protected String formattedDate; | ||
| public Deadline(String desc) { |
There was a problem hiding this comment.
Will it be better if there is a blank line and no space at line 11?
| LocalDate inputDate = LocalDate.parse(date); | ||
| this.formattedDate = inputDate.format(DateTimeFormatter.ofPattern("MMM d yyyy")); | ||
| this.type = "D"; | ||
| this.isDone = false; |
| */ | ||
| public class Event extends Task { | ||
| protected String from; | ||
| protected String to; |
There was a problem hiding this comment.
Would it be better to state explicitly that the two strings represent timestamp?
src/main/java/zoe/Event.java
Outdated
| this.from = str[1].split(" ",2)[1]; | ||
| this.to = str[2].split(" ", 2)[1]; | ||
| this.type = "E"; | ||
| this.isDone = isDoneNumber.equals("1"); |
There was a problem hiding this comment.
"this" might not be necessary here as there is no ambiguous reference.
| if (directory.exists() && file.exists()) { | ||
| BufferedReader reader = new BufferedReader(new FileReader(file)); | ||
| ArrayList<Task> loadedTasks = new ArrayList<Task>(); | ||
| String line; |
There was a problem hiding this comment.
The guideline would suggest not creating the object without initialization, but I'm not sure how to write better here tbh.
| */ | ||
| public class Task { | ||
| protected String description; | ||
| protected String type; |
There was a problem hiding this comment.
Is it better to have these attributes final? As I think they can be confirmed upon initialization.
| /** | ||
| * Records whether a task is done or not for data loading | ||
| */ | ||
| public int isDoneNumerical() { |
There was a problem hiding this comment.
I thought the return type was boolean. Is it better to start the name with a verb?
| /** | ||
| * Marks a task as done to reflect in zoe | ||
| */ | ||
| public void markAsDone() { |
There was a problem hiding this comment.
I like this naming as it is clearer than my version.
| /** | ||
| * Prints opening statement | ||
| */ | ||
| public String saysHi() { |
There was a problem hiding this comment.
Is it better to simply write sayHi()?
Assertions were implemented as a branch but not committed to before working on the CodingQuality branch. Let's commit this version with some CodingQuality implementation first as practice for pull requests. This way, I still get to see the effects of some merging conflicts and parallel pull requests.
There were some magic numbers in the code so this commits rectify that problem. Doing away with magic numbers makes it easier to debug in future and provides clarity in code.
Add rest of the CodeQuality changes.
Late commit for assertions.
src/main/java/zoe/TaskList.java
Outdated
| this.tasks.add(t); | ||
| } | ||
| /** | ||
| * Deletes a new task to the list, when delete(task) is called | ||
| * @param i | ||
| */ | ||
| public void delete(int i) { | ||
| tasks.remove(Integer.valueOf(i) - 1); |
There was a problem hiding this comment.
I feel there is an inconsistent usage of 'this'. Some methods use this referencing while others do not
| } | ||
|
|
||
| public String getStatusIcon() { | ||
| return (isDone ? "X" : " "); |
There was a problem hiding this comment.
Maybe you can consider using this.isDone as opposed to isDone to be more consistent with styling
| return (isDone ? "X" : " "); | |
| return (this.isDone ? "X" : " "); |
| } else { | ||
| if (command.equals("todo")) { | ||
| res += ui.todoDescription(); | ||
| } else { | ||
| res += ui.invalidCommand(); |
There was a problem hiding this comment.
I really like your method names and they are very descriptive!
…owing order: Todos, Events, Deadlines where Deadlines are sorted based on date.
…er input for better clarity. Also changed maximum size of chatbot text to allow for dynamic sizing based on output text.
# Conflicts: # data/SavedTasks.txt
…ata is processed for more flexible reading from file and neater presentation of data. Add more JUnit test to todo and event.
…. Change saving using bye to saving whenever a user input is parsed to avoid dataloss should the user choose to close the application using the "X" button.
…nd " with the empty space.
Change Ui picture so that it reflects new functionality as stated above
…ely advise the user on how to use it
…ource Update README to let user know that keyword in find <keyword> is case sensitive
No description provided.