-
Notifications
You must be signed in to change notification settings - Fork 270
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
[Weiye] iP #295
base: master
Are you sure you want to change the base?
[Weiye] iP #295
Conversation
Let's implement a skeletal version of Duke that starts by greeting the user, simply echoes commands entered by the user, and exits when the user types bye.
Let's add the ability to store whatever text entered by the user and display them back to the user when requested.
Let's, * move the conditional break in the infinite loop into the loop condition * remove String.format()
Lets, * add Task class to represent tasks * add the ability to mark tasks as done and the ability to change the status back to not done
As there are multiple types of tasks that have some similarity between them, implement classes Todo, Deadline and Event classes to inherit from a Task class. Let's, * add support for tracking three types of task
Use the input/output redirection technique to semi-automate the testing of Duke.
Teach Duke to deal with errors such as incorrect inputs entered by the user. Let's * add DukeException class
Let's * Add support for deleting tasks from the list
Let's: * use enum for types of tasks
Let's: * add abstract class for different commands
Let's: * add Storage class to facilitate reading and writing of list to file
Branch level 7
Teach duke to understand date and time. Let's * update due date and event time to LocalDateTime instead of String
Implement level-8 functionalities
Let's: * add TaskList class * add Ui class
Add JavaDoc comments to the code
# Conflicts: # src/main/META-INF/MANIFEST.MF
Let's: * add CFind class to handle the search function * add appropriate response after the search is done
Implement Level-9 functionalities
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 easy to read. However, there are a few minor coding standard. Just a few nits to fix.
src/main/duke/Duke.java
Outdated
this.parser = new Parser(); | ||
} | ||
|
||
public void run() throws DukeException{ |
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 it should be a white space before the open curly bracket. I noticed the same issue in several other places too.
import java.util.ArrayList; | ||
|
||
public class TaskList { | ||
private ArrayList<Task> 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.
I think the variable name should be in camelCase. I suggest it should be tasks
src/main/duke/commands/CBye.java
Outdated
import main.duke.Ui; | ||
import main.duke.enums.CommandType; | ||
|
||
public class CBye extends Command{ |
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 the name of class be CommandBye? I think it is more meaningful. Similar with other command names.
src/main/duke/commands/CFind.java
Outdated
this.findString = findString; | ||
} | ||
|
||
public String getFindString() { return this.findString; } |
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 suggest follow EgyptianStyle when we make a function which has one line only. I noticed the same issue in several other places too.
src/main/duke/io/Parser.java
Outdated
switch (userCommand) { | ||
case "bye": | ||
newCommand = new CBye(); | ||
break; | ||
case "list": | ||
newCommand = new CList(); | ||
break; | ||
case "mark": | ||
try { | ||
int markIndex = Integer.parseInt(inputArray[1]) - 1; | ||
newCommand = new CMark(markIndex); | ||
} catch (IndexOutOfBoundsException e) { | ||
throw new DukeException("Please specify the task you wish to mark."); | ||
} catch (NumberFormatException e) { | ||
throw new DukeException("Invalid index format."); | ||
} | ||
break; | ||
case "unmark": | ||
try { | ||
int unmarkIndex = Integer.parseInt(inputArray[1]) - 1; | ||
newCommand = new CUnmark(unmarkIndex); | ||
} catch (IndexOutOfBoundsException e) { | ||
throw new DukeException("Please specify the task you wish to unmark."); | ||
} catch (NumberFormatException e) { | ||
throw new DukeException("Invalid index format."); | ||
} | ||
break; | ||
case "delete": | ||
try { | ||
int deleteIndex = Integer.parseInt(inputArray[1]) - 1; | ||
newCommand = new CDelete(deleteIndex); | ||
} catch (IndexOutOfBoundsException e) { | ||
throw new DukeException("Please specify the task you wish to delete."); | ||
} catch (NumberFormatException e) { | ||
throw new DukeException("Invalid index format."); | ||
} | ||
break; | ||
case "todo": | ||
String todoDescription = String.join(" ", | ||
Arrays.copyOfRange(inputArray, 1, inputArray.length)); | ||
if (todoDescription.equals("")) { | ||
throw new DukeException("Please specify the description of the todo task."); | ||
} | ||
newCommand = new CTodo(todoDescription); | ||
break; | ||
case "deadline": | ||
if (!userInput.contains("/by")) { | ||
throw new DukeException("Please specify the due date using the /by keyword."); | ||
} else { | ||
try { | ||
int byIndex = Arrays.asList(inputArray).indexOf("/by"); | ||
String deadlineDescription = String.join(" ", | ||
Arrays.copyOfRange(inputArray, 1, byIndex)); | ||
String dueDate = String.join(" ", | ||
Arrays.copyOfRange(inputArray, byIndex + 1, inputArray.length)); | ||
LocalDateTime.parse(dueDate, DateTimeFormatter.ofPattern("yyyy-MM-dd kkmm")); | ||
// check if the date and time input is in the right format | ||
if (deadlineDescription.equals("") || dueDate.equals("")) { | ||
throw new DukeException("Please specify the description/due date of the deadline task."); | ||
} | ||
newCommand = new CDeadline(deadlineDescription, dueDate); | ||
} catch (DateTimeParseException e){ | ||
throw new DukeException("Please provide the date time in this format YYYY-MM-DD 0000"); | ||
} | ||
} | ||
break; | ||
case "event": | ||
if (!userInput.contains("/at")) { | ||
throw new DukeException("Please specify the date time using the /at keyword."); | ||
} else { | ||
try { | ||
int byIndex = Arrays.asList(inputArray).indexOf("/at"); | ||
String eventDescription = String.join(" ", | ||
Arrays.copyOfRange(inputArray, 1, byIndex)); | ||
String dateTime = String.join(" ", | ||
Arrays.copyOfRange(inputArray, byIndex + 1, inputArray.length)); | ||
LocalDateTime.parse(dateTime, DateTimeFormatter.ofPattern("yyyy-MM-dd kkmm")); | ||
// check if the date and time input is in the right format | ||
if (eventDescription.equals("") || dateTime.equals("")) { | ||
throw new DukeException("Please specify the description/date time of the event task."); | ||
} | ||
newCommand = new CEvent(eventDescription, dateTime); | ||
} catch (DateTimeParseException e){ | ||
throw new DukeException("Please provide the date time in this format YYYY-MM-DD 0000"); | ||
} | ||
} | ||
break; | ||
case "find": | ||
String findString = String.join(" ", | ||
Arrays.copyOfRange(inputArray, 1, inputArray.length)); | ||
newCommand = new CFind(findString); | ||
break; | ||
default: | ||
throw new DukeException("Sorry. I do not understand your input."); | ||
} | ||
return newCommand; |
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 there is no indentation for case clauses. I suggest you change the setting in the IDE.
|
||
import main.duke.enums.TaskType; | ||
|
||
public 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.
Should the class name Todo? 🤔
Use Gradle to automate some of the build tasks of the project
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 you adhered mostly to the coding standards!
String userCommand = inputArray[0]; | ||
Command newCommand; | ||
|
||
switch (userCommand) { |
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.
there should not be an indentation for case clauses to follow the coding standards
Add GUI supoport
Implement Level-10 functionalities
Improve code quality based on Code Quality topics
Use Assertions
Added cloning function for the tasks so that the previous state of the tasks list can be saved. Stack is used to store the previous state of the tasks list. Unifying all those duplicated code improves the code quality. As a step towards such unification, let's extract those duplicated code blocks into separate methods in their respective classes. Doing so will make the subsequent unification easier.
Add user guide
No description provided.