-
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
[shunjieee] iP #455
base: master
Are you sure you want to change the base?
[shunjieee] iP #455
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
The chatbot now take user inputs. Implementing echo functionality allows user to interact with the chatbot.
User inputs are tasks to be added into the list. Inputting "list" allows return of the tasks listed in chronological order. Let's use a data structure (i.e. ArrayList) to store the tasks. ArrayList allows random access of tasks in the array.
* commit '02c1ec31378fffb8932bc84b33ea9c024e97ce70': Add ToDo.java
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 👍
Overall, I find your code easy to read and most part follows the coding standards.
src/main/java/Duke.java
Outdated
import helperpackage.Deadline; | ||
import helperpackage.DukeException; | ||
import helperpackage.Event; | ||
import helperpackage.Task; | ||
import helperpackage.ToDo; | ||
|
||
import java.util.LinkedList; | ||
import java.util.NoSuchElementException; | ||
import java.util.Scanner; | ||
import java.util.StringTokenizer; |
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.
import helperpackage.Deadline; | |
import helperpackage.DukeException; | |
import helperpackage.Event; | |
import helperpackage.Task; | |
import helperpackage.ToDo; | |
import java.util.LinkedList; | |
import java.util.NoSuchElementException; | |
import java.util.Scanner; | |
import java.util.StringTokenizer; | |
import java.util.LinkedList; | |
import java.util.NoSuchElementException; | |
import java.util.Scanner; | |
import java.util.StringTokenizer; | |
import helperpackage.Deadline; | |
import helperpackage.DukeException; | |
import helperpackage.Event; | |
import helperpackage.Task; | |
import helperpackage.ToDo; |
/** Task description, also the user input */ | ||
protected String description; | ||
/** Status of the task, done or not done */ | ||
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.
I like how you name the Boolean variable 😃
src/main/java/Duke.java
Outdated
int index = Integer.parseInt(st.nextToken()); | ||
Task t = taskList.remove(index - 1); | ||
|
||
System.out.println("Noted, I've removed this task:"); | ||
System.out.println(" " + t.toString()); | ||
System.out.println("Now you have " + taskList.size() + " tasks in the list."); | ||
} |
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.
int index = Integer.parseInt(st.nextToken()); | |
Task t = taskList.remove(index - 1); | |
System.out.println("Noted, I've removed this task:"); | |
System.out.println(" " + t.toString()); | |
System.out.println("Now you have " + taskList.size() + " tasks in the list."); | |
} | |
int index = Integer.parseInt(st.nextToken()); | |
Task t = taskList.remove(index - 1); | |
System.out.println("Noted, I've removed this task:"); | |
System.out.println(" " + t.toString()); | |
System.out.println("Now you have " + taskList.size() + " tasks in the list."); | |
} |
src/main/java/Duke.java
Outdated
} | ||
|
||
// Level-3: mark & unmark | ||
public static void changeStatus(LinkedList<Task> taskList, String cmd, StringTokenizer st) throws IndexOutOfBoundsException, |
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 you use line wrapping at appropriate place of this line
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.
After reviewing your code for naming standards, no significant issues are identified. Nonetheless, there are a couple of spots where tweaking the names could make things a bit clearer. Keep up the great work! :)))
src/main/java/Duke.java
Outdated
// Level-4: ToDo, Deadline, Event | ||
public static void addTask(LinkedList<Task> taskList, String cmd, String userInput) throws DukeException { | ||
int firstSpaceIndex = userInput.indexOf(" "); | ||
String description = userInput.substring(firstSpaceIndex + 1); |
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.
Here the description
means the whole part following the command type, while in the Task class another description
means solely the description part of a task, which would be a little misleading or ambiguous. Would it be better to change the variable name here?
src/main/java/Duke.java
Outdated
public static void delete (LinkedList<Task> taskList, String cmd, StringTokenizer st) throws IndexOutOfBoundsException, | ||
NumberFormatException, NoSuchElementException { | ||
|
||
int index = Integer.parseInt(st.nextToken()); |
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.
The name index
doesn't explain its meaning accurately. Could you try to specify what the index stands for, like the firstSpaceIndex
in your code above?
This function allows a different formatting that is used in the text file, e.g. E | 1 | concert | Aug 2nd 4-6pm, as compared to the toString method. This format is also in accordance with how the text file will be read when the programme runs. Let's create a new function to give an output with a specific format
Data can be read from a text file when the program is initialised. The text file is updated when the program terminated. Let's * create an instance of File to locate the text file * handle errors when locating the file, i.e. file / path does not exist * read existing data from the file and add them into the taskList, if any * update the text file when the program terminates
* branch-Level-7: Add branch-Level-7 requirements Add toData() function
More functionalities can be used by storing the deadline as LocalDateTime as compared to String Let's * Create one formatter for user inputs * Create one formatter for output and reading from text file * Parse String into LocalDateTime * Handle error if the input does not match the formats
More functionalities can be used by storing the start time and end time as LocalDateTime as compared to String Let's * Create one formatter for user inputs * Create one formatter for output and reading from text file * Parse String into LocalDateTime * Handle error if the input does not match the formats
After changing Deadline.java and Event.java to use LocalDateTime instead of String to store date and time, Duke.java has to handle exceptions thrown by those two classes. Let's add try-catch block to handle exceptions thrown when creating an instance of Deadline or Event.
* branch-Level-8: Add branch-Level-8 requirements Change startAndEndTime from String to LocalDateTime Change deadline from String to LocalDateTime
The new format input allow easier parsing. Let's use StringTokeniser to read and process the user inputs. Using StringTokeniser allows a neater string processing as compared to slicing.
* master: Update format of input
* branch-A-JavaDoc: Update files to include JavaDocs and comments
* branch-A-CodingStandard: Update files to match coding standards # Conflicts: # src/main/java/common/DukeException.java # src/main/java/common/Parser.java # src/main/java/common/Storage.java
* branch-Level-9: Add Level-9 requirements Revert "Merge branch 'add-gradle-support' into branch-Level-9" Bump gradle and lib version Add Gradle support docs/README.md: Tweak document template # Conflicts: # src/main/java/Duke.java # src/main/java/common/Parser.java
* add-gradle-support: Update gradle filepath # Conflicts: # build.gradle
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, I found your code is easy to read. LGTM, just a few nits to fix.
For simplicity sake, perpahs line 72 "boolean isDone = Integer.parseInt(tasks[1]) == 1 ? true : false;
can be simplified to boolean isDone = Integer.parseInt(tasks[1]) == 1;
? Also, perhaps a more intuitive constant variable name here for "T, "D", and "E"?
Pardon my PR at this point of time.
This reverts commit e7e3cee.
* branch-Level-10: Revert "Add GUI and Level 10 requirement" Add GUI and Level 10 requirement
Assertions can allow programmers to detect possible bugs in the code. Let's add assertions after the main logic of add, delete, mark and unmark commands. Using assertion is preferable over exceptions in this situation as the errors are not due to invalid user input or the environment.
Add assertions for add, delete, mark and unmark
Minor adjustment made to improve code readability and quality, adhering to principles such as SLAP and KISS. Let's, * abstract duplicate codes * run checkstyleMain for any violations of conventions * Apply SLAP and KISS principle
Improve code quality
* 'master' of github.com:shunjieee/ip: Improve code quality
Allow user to sort the list base on the task's completion status. Let's * add SortCommand.java * update Parser.java to include "sort"
* 'master' of github.com:shunjieee/ip: Update README.md Update README.md Update README.md Update README.md Update README.md
Xavier, your
FinancialTask AdvisorXavier frees your mind
(I mean take your precious time)of having remember things you need to do. It's,fastSUPER DUPER FASTAll 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: