-
Notifications
You must be signed in to change notification settings - Fork 364
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
[Ong Wei Jie] ip #170
base: master
Are you sure you want to change the base?
[Ong Wei Jie] ip #170
Conversation
Replaced some if-else statements with case. Replaced remove() method in task class and subclasses to removeTask().
* branch-level-7: Add ability to save tasks in hard disk # Conflicts: # src/main/java/Duke.java
* commit '556af3f47a96b32898ab4cdbd65b16486a4871e8': Add Gradle support
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 your code is easy to understand 👍 Keep it up!
src/main/java/Duke.java
Outdated
DateTimeFormatter formatter = DateTimeFormatter.ofPattern("d/MM/yyyy HHmm"); | ||
LocalDateTime dateTime = LocalDateTime.parse(taskDate[1], formatter); | ||
String dateTimeString = dateTime.format(DateTimeFormatter.ofPattern("MMM d yyyy hhmma")); | ||
input = taskDate[0] + "(by:" + dateTimeString + ")"; | ||
newTask = new Deadline(input); | ||
break; | ||
case "event" : |
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.
Just a minor coding standard violation but I think there should not be a white space between the "event" condition and the colon
src/main/java/Storage.java
Outdated
break; | ||
case "D" : | ||
this.actions.add(new Deadline(taskName)); | ||
break; |
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 you would like to insert a default case to make it clear that it is the end of the switch statement?
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 fould your code easy to read. Just ensure JavaDocs end off with fullstops.
src/main/java/duke/Parser.java
Outdated
* @throws Missing if there is missing information regarding date and time | ||
* @throws WrongKeyWord if there is a missing key command | ||
*/ | ||
public static String understandInput(String input, String[] input_arr, TaskList tasks, Storage storage) throws Missing, WrongKeyWord { |
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 both input and input_arr both need to be passed in to the function? Could you obtain one from the other?
src/main/java/duke/Parser.java
Outdated
public class Parser { | ||
|
||
/** | ||
* Returns reply string based on. |
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.
Is the fullstop here intended?
src/main/java/duke/Storage.java
Outdated
* @param filePath path of where the file is. | ||
*/ | ||
public Storage(String filePath) { | ||
this.path = "./src/main/data/duke.txt"; |
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.
Should this make use of the filePath passed into the constructor 🤔
src/main/java/duke/Ui.java
Outdated
* Class that prints output for the user. | ||
*/ | ||
public class Ui { | ||
private Scanner r; |
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.
Maybe a more descriptive name for the scanner would make it clearer.
package duke.dukeexceptions; | ||
|
||
/** | ||
* An exception that throws when TaskList is empty. | ||
*/ | ||
public class TaskListEmpty extends DukeException{ | ||
|
||
/** | ||
* Constructor for the Exception. | ||
* | ||
* @param s string to be thrown during exception. | ||
*/ | ||
public TaskListEmpty(String s) { | ||
super(s); | ||
} | ||
|
||
/** | ||
* {@inheritDoc} | ||
* | ||
* @return a warning message when there is missing information. | ||
*/ | ||
@Override | ||
public String getMessage() { | ||
return "☹ hmm strange the list is missing and so are my cookies?!?"; | ||
} | ||
} |
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 use of various duke exceptions for error handling 👍
return "Noted I've removed this task:\n " | ||
+ this.status() + "\n" + "Now you have " | ||
+ super.total + " 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.
Clean line breaks, improves readability 👍
src/main/java/duke/task/ToDo.java
Outdated
/** | ||
* Returns the updates on removal | ||
* | ||
* @return string regarding removal of a ToDo | ||
*/ |
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.
Maybe consider adding a fill stop to the end of JavaDocs sentences
There was a complicated "if else" expression to determind the types of task as there were a few or expressions. Breaking down the "if else" expression into their own individual expressions improved the readability and code quality.
Branch a assertions
Revert "Branch a assertions"
* 'master' of https://github.com/ongweijie7/ip: Revert "Branch a assertions" Add Assertions and also catch statements to catch various exceptions Add assertion and created .jar file
* origin/master: Revert "Branch a assertions" Add Assertions and also catch statements to catch various exceptions Add assertion and created .jar file
* origin/branch-A-CodeQuality: Improve code quality
…own. Added catch statements to catch exceptions because they could be thrown when user gave missing commands. Exceptions that occur when there is a missing time/date or wrong format given will be caught and I will throw a Missing DukeException. It is done this way since this error is specific to a missing command. The error message for Missing exception is also improved as I realised the missing command is also used when there is no information on the task name given
There was a complicated "if else" expression to determind the types of task as there were a few or expressions to check for all types of task. Breaking down the "if else" expression into their own individual expressions improved the readability and code quality. This ensures that there are seperate conditional expressions to consider each variation of task added, allowing for easier understanding of code.
…own. Added catch statements to catch exceptions because they could be thrown when user gave missing commands. Exceptions that occur when there is a missing time/date or wrong format given will be caught and I will throw a Missing DukeException. It is done this way since this error is specific to a missing command. The error message for Missing exception is also improved as I realised the missing command is also used when there is no information on the task name given
* commit '306bf4fd7b532958292332396c6a4e359c29ac73': Remove 3 empty lines Add a few catch statements in Parser and edited the error message thrown. Improve code quality
Abstracted away the marking, unmarking and deleting of tasks. Also make it such that storage will keep track of done tasks. TaskList should handle the marking of tasks instead of letting tasks handle it themselves. It is done this way because Storage needs to constantly update the textfile.
Needs to be changed to make things easier for user especially for command line users Have shortened the first command to a simple capital letter. This ensures that actual commands can be differentiated from accidental inputs.
DukePro frees your mind of having to remember things you need to do. It's,
FastSUPER FAST to use for CLI usersALL YOU NEED TO DO IS TO download it from here.
Features:
If you Java programmer, you can use it to practice Java too. Here's the
main
method: