-
Notifications
You must be signed in to change notification settings - Fork 484
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
[Luah Jun Yang} iP #571
base: master
Are you sure you want to change the base?
[Luah Jun Yang} iP #571
Conversation
…ut text when 'list' is input
…ete method to Frenchie.java, level 3 completed
… for todo, event, deadline. Level 4 Complete.
…w. Level 5 completed up till basic exceptions.
…st instead of List
src/main/java/Frenchie.java
Outdated
} else if (input.contains("delete")){ | ||
String[] parts = input.split(" "); | ||
int index = Integer.parseInt(parts[1]) - 1; | ||
Task target_task = frenchie.tasks.get(index); | ||
frenchie.deleteTask(index); | ||
System.out.println("____________________________________________________________\n" + | ||
"Noted. I've removed this task: \n" + | ||
target_task.toString() + "\n" + | ||
"Now you have " + frenchie.getNumOfTasks() + " tasks in the list.\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.
As the number of keywords increase, perhaps you might want to start looking at enums to have a collection of your keywords!
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.
Seconded. In addition, perhaps you may wish to divide all the logic in your main
method into several other methods, as the complexity of your program increases.
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 OOP and inheritance for the subclasses of Task
. You can still improve on your coding style and your use of SLAP.
src/main/java/Frenchie.java
Outdated
this.tasks = new ArrayList<>(); | ||
} | ||
|
||
public void addTask(Task NEW_TASK) { |
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 Task parameter is a variable, not a constant. So it should be in camelCase instead, i.e. newTask
.
src/main/java/Frenchie.java
Outdated
} | ||
|
||
public void completeTask(int index) { | ||
tasks.get(index).mark_as_completed(); |
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.
Please follow the camelCase convention for methods, i.e. markAsCompleted
.
src/main/java/Frenchie.java
Outdated
} else if (input.contains("delete")){ | ||
String[] parts = input.split(" "); | ||
int index = Integer.parseInt(parts[1]) - 1; | ||
Task target_task = frenchie.tasks.get(index); | ||
frenchie.deleteTask(index); | ||
System.out.println("____________________________________________________________\n" + | ||
"Noted. I've removed this task: \n" + | ||
target_task.toString() + "\n" + | ||
"Now you have " + frenchie.getNumOfTasks() + " tasks in the list.\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.
Seconded. In addition, perhaps you may wish to divide all the logic in your main
method into several other methods, as the complexity of your program increases.
src/main/java/Frenchie.java
Outdated
+ "|____/ \\__,_|_|\\_\\___|\n"; | ||
System.out.println("Hello from\n" + logo); */ | ||
Frenchie frenchie = new Frenchie(); | ||
String skeleton = "____________________________________________________________\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.
When breaking up a statement into multiple lines, the +
operator should be on a new line.
src/main/java/ToDo.java
Outdated
@@ -0,0 +1,11 @@ | |||
class ToDo extends Task { |
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 should add the public
access modifier to your class definition?
src/main/java/Event.java
Outdated
@@ -0,0 +1,16 @@ | |||
public class Event extends Task{ |
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 leave a whitespace between your definition and the curly braces, as in
public class Event extends Task {
src/main/java/Frenchie.java
Outdated
break; | ||
} else if (input.equals("list")) { //Checking if user is looking to list all tasks | ||
frenchie.listTasks(); | ||
} else if (input.contains("mark")) { //Checking if user input is to mark/unmark tasks |
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 doesn't check if the input is either mark
or unmark
. If a user inputs markzzzzzz
, this will still be treated as unmark
even though it is an invalid command, because the input still contains the substring mark
. To avoid similar problems like this, perhaps you should use Java Enums to ensure input validity.
…tors for Event and Deadline classes as well as save and load methods for Frenchie
…he scanner is closed
LocalDateTime storedEndTime = LocalDateTime.parse(endTime, inputFormatter); | ||
DateTimeFormatter desiredFormatter = DateTimeFormatter.ofPattern("dd/MM/yyyy HH:mm"); | ||
String constructorStartTime = storedStartTime.format(desiredFormatter); | ||
String constructorEndTime = storedEndTime.format(desiredFormatter); |
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 think this part of your code and be improved? Maybe try to use some other methods?😊
…rnMatchTasks() method to TaskList.java. Level-9 Completed.
Removed unused methods and corrected the order of import statements to be in the correct lexographical order. Fixed the error of tasks not loading from frenchie.txt by calling storage.load() before parsing the input String.
Improving on Code Quality
Added help as an enum in Command and included 'help' as a case for the Parser class. Added a quick start guide that will be presented to the user when user inputs 'help' as the command.
Add help function
Frenchie
Frenchie keeps track of all tasks in your life in a fuss-free CLI. It is:
All you need to start using Frenchie is:
Features:
For those of you who code, Frenchie is written in java and you can experiment with it to add your own features!
Here is the main() method of Frenchie: