-
Notifications
You must be signed in to change notification settings - Fork 454
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
[Aakansha Narain] iP #491
base: master
Are you sure you want to change the base?
[Aakansha Narain] iP #491
Changes from 10 commits
d839859
ff47a7a
7358409
3147541
5869554
9ff98b5
abe05eb
afabe7d
a82ef81
1a56d9d
15af52e
feca504
e3694ff
8c70f0d
edfb3f6
257ac69
ab089f4
fc54428
3109605
acf0641
98b0856
20af5bb
3c0c2b5
7192bbb
c37b6a3
4619db6
c228d14
16385f3
94c3a0a
9370619
6df5f8c
227584f
f576001
0f34752
8a175e0
c4e7b8f
4c29fb3
e5d7624
896873a
7f2eaac
3c4eac7
1a66261
9163300
8e9ec2f
6b49768
b90d9b5
52a8bb9
43f2042
9126f32
f073024
e696299
e65284f
ca20ffe
9ca2212
dac5432
589e068
7fceaae
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,10 +1,64 @@ | ||
//Solution below slightly adapted from https://github.com/Wincenttjoi/CS2103T-duke-chatbot/blob/master/src/main/java/duke/Duke.java | ||
|
||
import duke.*; | ||
|
||
public class Duke { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You could also add some comments here! I think this would allow you to communicate your code effectively! |
||
|
||
private Ui ui; | ||
private TaskList taskList; | ||
private boolean exit; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You could change this to a more boolean sounding name! |
||
|
||
public Duke () { | ||
this.ui = new Ui(); | ||
this.taskList = new TaskList(); | ||
exit = false; | ||
} | ||
|
||
public enum InputCommands { | ||
bye, list, done, delete, todo, deadline, event | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since these are going to be unchanged, you could possibly rename to be capitalised? For example, BYE, LIST, DONE etc. However, i like the naming of the enum InputCommands here! |
||
} | ||
|
||
public void start () { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think you do not need to add a space after the method name here! just start() would do |
||
System.out.println(ui.greet()); | ||
while (!exit) { | ||
try { | ||
String userCommand = ui.echoCommand(); | ||
|
||
if (userCommand.equals("bye")) { | ||
System.out.println(ui.exit()); | ||
exit = true; | ||
} else if (userCommand.equals("list")) { | ||
System.out.println(ui.retrieveList()); | ||
} else if (userCommand.startsWith("done")) { | ||
InputHandler doneInputHandler = new DoneInput(ui, taskList); | ||
System.out.println(doneInputHandler.handle(userCommand)); | ||
} else if (userCommand.startsWith("delete")) { | ||
InputHandler deleteInputHandler = new DeleteInput(ui, taskList); | ||
System.out.println(deleteInputHandler.handle(userCommand)); | ||
} else if (userCommand.startsWith("todo")) { | ||
InputHandler todoInputHandler = new TodoInput(ui, taskList); | ||
System.out.println(todoInputHandler.handle(userCommand)); | ||
} else if (userCommand.startsWith("deadline")) { | ||
InputHandler deadlineInputHandler = new DeadlineInput(ui, taskList); | ||
System.out.println(deadlineInputHandler.handle(userCommand)); | ||
} else if (userCommand.startsWith("event")) { | ||
InputHandler eventInputHandler = new EventInput(ui, taskList); | ||
System.out.println(eventInputHandler.handle(userCommand)); | ||
} else if (userCommand.equals("listInputs")) { | ||
for (InputCommands inputs : InputCommands.values()) { | ||
System.out.println(inputs); | ||
} | ||
} | ||
else { | ||
throw new UnknownInputException("error"); | ||
} | ||
} catch (DukeException e) { | ||
ui.getError(e.getMessage()); | ||
} | ||
} | ||
} | ||
|
||
public static void main(String[] args) { | ||
String logo = " ____ _ \n" | ||
+ "| _ \\ _ _| | _____ \n" | ||
+ "| | | | | | | |/ / _ \\\n" | ||
+ "| |_| | |_| | < __/\n" | ||
+ "|____/ \\__,_|_|\\_\\___|\n"; | ||
System.out.println("Hello from\n" + logo); | ||
new Duke().start(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i think you could explicitly close Duke after this by adding a System.close() here! |
||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,16 @@ | ||
package duke; | ||
|
||
public class Deadline extends Task { | ||
|
||
protected String by; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i think this variable should be private even if it is only going to be accessed within the package! you could add a getter to retrieve this variable |
||
|
||
public Deadline(String description, String by) { | ||
super(description); | ||
this.by = by; | ||
} | ||
|
||
@Override | ||
public String toString() { | ||
return "[D]" + super.toString() + " (by: " + by + ")"; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,24 @@ | ||
package duke; | ||
|
||
import duke.Deadline; | ||
|
||
public class DeadlineInput extends InputHandler { | ||
|
||
public DeadlineInput(Ui ui, TaskList taskList) throws DukeException { | ||
super(ui, taskList); | ||
} | ||
|
||
@Override | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think you could refactor your toString method here! or possibly store the instantiated objects as variables instead! |
||
public String handle(String input) throws EmptyDescriptionException { | ||
if (input.length() == 8) { | ||
throw new EmptyDescriptionException("error" ); | ||
} | ||
int charIndex = input.indexOf("/" ); | ||
int byIndex = charIndex + 4; | ||
String by = input.substring(byIndex); | ||
String task = input.substring(9, charIndex - 1); | ||
Task deadlineTask = new Deadline(task, by); | ||
taskList.add(deadlineTask); | ||
return ui.addedTask(deadlineTask); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,23 @@ | ||
package duke; | ||
|
||
public class DeleteInput extends InputHandler { | ||
|
||
public DeleteInput(Ui ui, TaskList taskList) { | ||
super(ui, taskList); | ||
} | ||
|
||
@Override | ||
public String handle(String input) throws EmptyDescriptionException { | ||
if (input.length() == 6) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not too sure whether i understand the reasoning behind using length() here. Could you comment on this? |
||
throw new EmptyDescriptionException("error"); | ||
} | ||
|
||
char taskIndex = input.charAt(7); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i like this separation of lines. allows me to understand the flow of logic slightly better. |
||
int index = Integer.parseInt(String.valueOf(taskIndex)); | ||
|
||
Task taskAtIndex = taskList.getTask(index); | ||
|
||
taskList.removeTask(index); | ||
return ui.deletedTask(taskAtIndex); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,24 @@ | ||
package duke; | ||
|
||
public class DoneInput extends InputHandler { | ||
|
||
public DoneInput(Ui ui, TaskList taskList) throws DukeException { | ||
super(ui, taskList); | ||
} | ||
|
||
@Override | ||
public String handle (String input) throws EmptyDescriptionException { | ||
if (input.length() == 4) { | ||
throw new EmptyDescriptionException("error"); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it's great to have different Exceptions here! really improves the OO-ness of the project. |
||
} | ||
|
||
char taskIndex = input.charAt(5); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think you could rename the variable here, to something like indexOfTask? |
||
int index = Integer.parseInt(String.valueOf(taskIndex)); | ||
|
||
Task taskAtIndex = taskList.getTask(index); | ||
taskAtIndex.markAsDone(); | ||
|
||
return ui.doneTask(taskAtIndex); | ||
} | ||
|
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
package duke; | ||
|
||
public class DukeException extends Exception{ | ||
|
||
public DukeException (String e) { | ||
super(e); | ||
} | ||
|
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
package duke; | ||
|
||
public class EmptyDescriptionException extends DukeException{ | ||
|
||
public EmptyDescriptionException(String e) { | ||
super("Error: The description of that command cannot be empty. Please enter a description."); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,17 @@ | ||
package duke; | ||
|
||
public class Event extends Task { | ||
|
||
private String at; | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You could remove the extra empty line |
||
|
||
public Event(String description, String at) { | ||
super(description); | ||
this.at = at; | ||
} | ||
|
||
@Override | ||
public String toString() { | ||
return "[E]" + super.toString() + " (at: " + at + ")"; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,27 @@ | ||
package duke; | ||
|
||
import duke.Event; | ||
|
||
public class EventInput extends InputHandler { | ||
|
||
|
||
public EventInput(Ui ui, TaskList taskList) throws DukeException { | ||
super(ui, taskList); | ||
} | ||
|
||
public String handle(String input) throws EmptyDescriptionException { | ||
if (input.length() == 5) { | ||
throw new EmptyDescriptionException("error" ); | ||
} | ||
int charIndex = input.indexOf("/" ); | ||
int atIndex = charIndex + 4; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I like how you named your variables here! (Easy to understand) |
||
|
||
String at = input.substring(atIndex); | ||
String task = input.substring(6, charIndex - 1); | ||
|
||
Task eventTask = new Event(task, at); | ||
taskList.add(eventTask); | ||
|
||
return ui.addedTask(eventTask); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,14 @@ | ||
package duke; | ||
|
||
public abstract class InputHandler { | ||
protected Ui ui; | ||
protected TaskList taskList; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You could add a line between? (To keep your coding style consistent, same thing with couple of other files) |
||
|
||
public InputHandler(Ui ui, TaskList taskList) { | ||
this.taskList = taskList; | ||
this.ui = ui; | ||
} | ||
|
||
public abstract String handle(String input) throws EmptyDescriptionException; | ||
|
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,25 @@ | ||
package duke; | ||
|
||
public class Task { | ||
protected String description; | ||
protected boolean isDone; | ||
|
||
public Task(String description) { | ||
this.description = description; | ||
this.isDone = false; | ||
} | ||
|
||
public String getStatusIcon() { | ||
return (isDone ? "X" : " "); // mark done task with X | ||
} | ||
|
||
public void markAsDone () { | ||
this.isDone = true; | ||
} | ||
|
||
@Override | ||
public String toString () { | ||
return "[" + getStatusIcon() + "] " + this.description; | ||
} | ||
|
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,28 @@ | ||
package duke; | ||
|
||
import java.util.ArrayList; | ||
|
||
public class TaskList { | ||
private static ArrayList<Task> todoList; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You could rename this to tasks to keep it consistent with the coding standards provided by prof! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this could also be instance level (non-static) rather than class level! I think it would be easier to keep track of the collection this way. |
||
|
||
public TaskList() { | ||
todoList = new ArrayList<>(); | ||
} | ||
|
||
public static ArrayList<Task> getTodoList () { | ||
return todoList; | ||
} | ||
|
||
public void add (Task task) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. again, as with other lines of code, you could remove the space between the method name and parameter parentheses |
||
todoList.add(task); | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I like how you named your variables with verbs, easy to understand at one glance. |
||
|
||
public Task getTask (int index) { | ||
return todoList.get(index - 1); | ||
} | ||
|
||
public void removeTask (int index) { | ||
todoList.remove(index - 1); | ||
} | ||
|
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
package duke; | ||
|
||
public class Todo extends Task { | ||
|
||
public Todo(String description) { | ||
super(description); | ||
} | ||
|
||
@Override | ||
public String toString() { | ||
return "[T]" + super.toString(); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,20 @@ | ||
package duke; | ||
|
||
public class TodoInput extends InputHandler { | ||
|
||
public TodoInput(Ui ui, TaskList taskList) throws DukeException { | ||
super(ui, taskList); | ||
} | ||
|
||
public String handle(String input) throws EmptyDescriptionException { | ||
if (input.length() == 4) { | ||
throw new EmptyDescriptionException("error"); | ||
} | ||
|
||
String task = input.substring(5); | ||
Task toDoTask = new Todo(task); | ||
|
||
taskList.add(toDoTask); | ||
return ui.addedTask(toDoTask); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,61 @@ | ||
package duke; | ||
|
||
import java.util.ArrayList; | ||
import java.util.Scanner; | ||
|
||
public class Ui { | ||
|
||
private static final String GREET = "Hi there! My name's Duke, how can I help you today?"; | ||
private static final String EXIT = "Bye! See you next time!"; | ||
|
||
private Scanner sc; | ||
|
||
public Ui() { | ||
this.sc = new Scanner(System.in); | ||
} | ||
|
||
public String greet () { | ||
return GREET; | ||
} | ||
|
||
public String exit () { | ||
return EXIT; | ||
} | ||
|
||
public String echoCommand () { | ||
return sc.nextLine(); | ||
} | ||
|
||
public String retrieveList() { | ||
ArrayList<Task> list = TaskList.getTodoList(); | ||
if (list.size() == 0) { | ||
return "You currently have no tasks!"; | ||
} else { | ||
String userList = ""; | ||
for (Task task : list) { | ||
userList = userList + (list.indexOf(task) + 1) + ". " + task.toString() + "\n"; | ||
} | ||
return "Here are the tasks in your list:\n" + userList; | ||
} | ||
} | ||
|
||
public String addedTask(Task task) { | ||
return String.format("Got it. I've added this task:\n %s\nNow you have %d tasks in the list.", | ||
task.toString(), TaskList.getTodoList().size()); | ||
} | ||
|
||
public String doneTask (Task task) { | ||
return String.format("Nice! I've marked this task as done:\n %s", task.toString()); | ||
} | ||
|
||
public String deletedTask(Task task) { | ||
return String.format("Noted. I've removed this task:\n %s\nNow you have %d tasks in the list.", | ||
task.toString(), TaskList.getTodoList().size()); | ||
} | ||
|
||
public void getError(String e) { | ||
System.out.println(e); | ||
} | ||
|
||
|
||
} |
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 you be including the individual packages here rather than a wildcard?