[Ryan-Reno] iP #532
[Ryan-Reno] iP #532
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
src/main/java/Chatbot.java
Outdated
| } | ||
|
|
||
| if (instruction[0].equals("delete")) { | ||
| this.delete(instruction); |
There was a problem hiding this comment.
Consider removing the 'this' reference since there is inconsistent styling with addToDo among other instructions
| this.delete(instruction); | |
| delete(instruction); |
src/main/java/Chatbot.java
Outdated
| public class Chatbot { | ||
| private Scanner scanner = new Scanner(System.in); | ||
| private String currentInput; | ||
| private ArrayList<Task> list; |
There was a problem hiding this comment.
Suggestion: Why not have a Tasks class to encapsulate the ArrayList objects? That way it makes the code in Chatbot.java a lot less dense, and you can abstract away most of the complexity into that class.
There was a problem hiding this comment.
Sorry I don't quite get what you mean. Isn't the point of the Task class to encapsulate the ArrayList objects? If so, doesn't my Task class behave the same with the Tasks class you suggested?
src/main/java/Lite.java
Outdated
| @@ -0,0 +1,15 @@ | |||
| public class Lite { | |||
| public static void main(String[] args) { | |||
| String logo = " LLLLL IIIIIIIII TTTTTTTTTT EEEEEEEEEE \n" + | |||
There was a problem hiding this comment.
This is super cool and I really enjoy it :D
There was a problem hiding this comment.
Overall Good attempt but method names could use more consistent naming. UI class is currently handling too many functions and could be split up more. Exception handling is a bit weird as it doesn't actually catch any exceptions. LiteException class should extend Exception so you can actually use try catch blocks to handle exceptions.
There was a problem hiding this comment.
Method Names could be clearer. parse() could be renamed into executeCommand() instead.
Methods like find, delete, mark addDeadline, addEvent could be renamed into findTask, deleteTask, markTaskAsDone, addDeadlineTask, addEventTask for more clarity.
Overall not a bad attempt however more OOP could be used. The Ui class is currently parsing input strings, processing business logic, and handling output all at the same time. You could separate the command parsing into a different class and allow the Ui class to solely handle text generation
src/main/java/lite/util/Printer.java
Outdated
There was a problem hiding this comment.
Method names seem inconsistent. toDoException could be renamed to printToDoException. successfulFind and unsuccessfulFind could be renamed to printFound() and printNotFound(). Since the Printer class solely handles output, methods should have consistent naming.
Also, it seems redundant to have methods to print Exceptions here when you're also handling exceptions in LiteExceptions.
There was a problem hiding this comment.
Thanks for this feedback! Was not thinking properly when creating the methods and simply put it within the Printer class.
src/main/java/lite/Ui.java
Outdated
| } else { | ||
| LiteException.invalidInput(); | ||
| } | ||
| return false; | ||
| } | ||
|
|
||
| private void find (String instruction[], TaskList tasks) { |
There was a problem hiding this comment.
method name could be clearer for example findTask() instead of just find()
| /** | ||
| * Outputs the string representation of the TaskList | ||
| */ | ||
| public void taskString() { |
There was a problem hiding this comment.
toString() would be better and follow naming conventions
There are several methods in Ui where tasks will be added and deleted. As a result, the size of the tasklist will go up and down accordingly. The usage of assert will ensure that the size of these will adjust according to what task has been presented, whether it is a delete or add.
The parsing method within Ui is put into its own class called Parser. Its task is to receive the input and determine which task the input is associated with
Branch a code quality
Added a constraint that does not allow tasks that has been added to be added again into the Task List. Tasks with different types (Event, Todo, and Deadline) with the same description will be considered a different task. Two deadline tasks will also be considered different if their deadlines do not end at the same time.
Detect duplicate tasks
LITE
Lite is the next generation's ToDo List! It's :
EASYSUPER EASY to learn and useAll you need to do is,
And it is COMPLETELY FREE!
Features :
If you are a Java programmer, you can use it to practice Java too. Here's the main method: