-
Notifications
You must be signed in to change notification settings - Fork 438
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
[Nguyen D.Q. Minh] iP #450
base: master
Are you sure you want to change the base?
Changes from 9 commits
a8ea12b
769ce65
e232267
2f70ea7
f018610
8042b02
d840068
d7b4523
882c2e0
f3e9e81
3024bed
f2688f4
082964e
fbe073e
95671b3
371dc42
0801bc8
5a44c6d
398480c
e9d74de
739d80a
955f166
068d38f
bf107c0
20471ee
cd2afb4
52c8a57
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 |
---|---|---|
@@ -0,0 +1 @@ | ||
0 |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,18 @@ | ||
public class Deadline extends Task { | ||
protected String by; | ||
|
||
public Deadline(String description, String by) { | ||
super(description); | ||
this.by = by; | ||
} | ||
|
||
public Deadline(String description, boolean isDone, String by) { | ||
super(description, isDone); | ||
this.by = by; | ||
} | ||
|
||
@Override | ||
public String toString() { | ||
return String.format("[D]%s (by: %s)", super.toString(), this.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 like the clean code here. Is it perhaps better to rename the variable "by" to something clearer? |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,10 +1,87 @@ | ||
import java.nio.file.Path; | ||
import java.nio.file.Paths; | ||
|
||
public class Duke { | ||
public static void main(String[] args) { | ||
String logo = " ____ _ \n" | ||
+ "| _ \\ _ _| | _____ \n" | ||
+ "| | | | | | | |/ / _ \\\n" | ||
+ "| |_| | |_| | < __/\n" | ||
+ "|____/ \\__,_|_|\\_\\___|\n"; | ||
System.out.println("Hello from\n" + logo); | ||
private TaskList tasks; | ||
private Storage storage; | ||
private UI ui; | ||
|
||
public Duke() { | ||
ui = new UI(); | ||
tasks = new TaskList(); | ||
String directory = System.getProperty("user.dir"); | ||
Path filePath = Paths.get(directory, "data", "data.txt"); | ||
storage = new Storage(filePath); | ||
try { | ||
tasks = new TaskList(storage.loadData()); | ||
} catch (Exception e) { | ||
ui.displayError(e); | ||
} | ||
} | ||
|
||
public void execute() throws Exception { | ||
ui.greetings(); | ||
boolean isHalted = false; | ||
while (!isHalted) { | ||
String command = ui.readCommand(); | ||
Parser parser = new Parser(command); | ||
String commandType = parser.getCommandType(); | ||
switch (commandType) { | ||
case "bye": { | ||
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. Shouldn't there be no indentation for case clauses? |
||
isHalted = true; | ||
break; | ||
} | ||
case "list": { | ||
ui.displayMessage("Here are the tasks in your list:"); | ||
for (int i = 0; i < tasks.size(); ++i) { | ||
ui.displayFormat("%d. %s\n", i + 1, tasks.get(i)); | ||
} | ||
break; | ||
} | ||
case "done": { | ||
int index = parser.getIndex(); | ||
if (index >= tasks.size()) { | ||
throw new DukeException(":( OOPS!!! Task index not found."); | ||
} | ||
tasks.get(index).markAsDone(); | ||
ui.displayMessage("Nice! I've marked this task as done:"); | ||
ui.displayMessage(tasks.get(index).toString()); | ||
break; | ||
} | ||
case "delete": { | ||
int index = parser.getIndex(); | ||
if (index >= tasks.size()) { | ||
throw new DukeException(":( OOPS!!! Task index not found."); | ||
} | ||
ui.displayMessage("Noted. I've removed this task:"); | ||
ui.displayMessage(tasks.get(index).toString()); | ||
tasks.removeTask(index); | ||
ui.displayFormat("Now you have %d tasks in the list.\n", tasks.size()); | ||
break; | ||
} | ||
case "add": { | ||
Task newTask = parser.getTask(); | ||
tasks.addTask(newTask); | ||
ui.displayMessage("Got it. I've added this task:"); | ||
ui.displayMessage(newTask.toString()); | ||
ui.displayFormat("Now you have %d tasks in the list\n", tasks.size()); | ||
break; | ||
} | ||
default: { | ||
throw new Exception("Unexpected error"); | ||
} | ||
} | ||
} | ||
|
||
try { | ||
storage.saveData(tasks.getTasks()); | ||
} catch(Exception e) { | ||
ui.displayError(e); | ||
} | ||
ui.bye(); | ||
} | ||
|
||
public static void main(String[] args) throws Exception { | ||
new Duke().execute(); | ||
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. Good usage of OOP principles! |
||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
public class DukeException extends Exception { | ||
public DukeException(String message) { | ||
super(message); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,18 @@ | ||
public class Event extends Task { | ||
protected 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. Similar to Deadline class, could the "at" variable name be changed to something more descriptive? |
||
|
||
public Event(String description, String at) { | ||
super(description); | ||
this.at = at; | ||
} | ||
|
||
public Event(String description, boolean isDone, String at) { | ||
super(description, isDone); | ||
this.at = at; | ||
} | ||
|
||
@Override | ||
public String toString() { | ||
return String.format("[E]%s (at: %s)", super.toString(), this.at); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,81 @@ | ||
public class Parser { | ||
private Task task; | ||
private String commandType; | ||
private int index; | ||
|
||
public Parser(String command) { | ||
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. Is there too much usage of if-else loops here? |
||
this.commandType = "undefined"; | ||
try { | ||
if (command.equals("bye")) { | ||
this.commandType = "bye"; | ||
} else if (command.equals("list")) { | ||
this.commandType = "list"; | ||
} else if (command.startsWith("done")) { | ||
if (command.length() < 5) { | ||
throw new DukeException(":( OOPS!!! Task index not found."); | ||
} | ||
this.commandType = "done"; | ||
this.index = Integer.parseInt(command.substring(5)) - 1; | ||
} else if (command.startsWith("delete")) { | ||
if (command.length() < 7) { | ||
throw new DukeException(":( OOPS!!! Task index not found."); | ||
} | ||
this.commandType = "delete"; | ||
this.index = Integer.parseInt(command.substring(7)) - 1; | ||
} else { | ||
Task newTask = null; | ||
if (command.startsWith("todo")) { | ||
if (command.length() < 5) { | ||
throw new DukeException(":( OOPS!!! The description of a todo cannot be empty."); | ||
} | ||
String description = command.substring(5); | ||
newTask = new Todo(description); | ||
} else if (command.startsWith("deadline")) { | ||
if (command.length() < 9) { | ||
throw new DukeException(":( OOPS!!! The description of a deadline cannot be empty."); | ||
} | ||
String rest = command.substring(9); | ||
int by_position = rest.indexOf("/by "); | ||
if (by_position == -1) { | ||
throw new DukeException(":( OOPS!!! Deadline time specification not found."); | ||
} | ||
String description = rest.substring(0, by_position - 1); | ||
String by = rest.substring(by_position + 4); | ||
newTask = new Deadline(description, by); | ||
} else if (command.startsWith("event")) { | ||
if (command.length() < 6) { | ||
throw new DukeException(":( OOPS!!! The description of an event cannot be empty."); | ||
} | ||
String rest = command.substring(6); | ||
int at_position = rest.indexOf("/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. Should underscore be used for naming here? According to the coding standards, underscores may be used in test method names and for constant names that are all caps (eg, MAX_ITERATIONS). |
||
if (at_position == -1) { | ||
throw new DukeException(":( OOPS!!! Event time specification not found."); | ||
} | ||
String description = rest.substring(0, at_position - 1); | ||
String at = rest.substring(at_position + 4); | ||
newTask = new Event(description, at); | ||
} | ||
|
||
if (newTask == null) { | ||
throw new DukeException(":( OOPS!!! I'm sorry, but I don't know what that means :-("); | ||
} | ||
this.commandType = "add"; | ||
this.task = newTask; | ||
} | ||
} catch(DukeException e) { | ||
System.out.println(e.getMessage()); | ||
} | ||
} | ||
|
||
public int getIndex() { | ||
return this.index; | ||
} | ||
|
||
public Task getTask() { | ||
return this.task; | ||
} | ||
|
||
public String getCommandType() { | ||
return this.commandType; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,45 @@ | ||
import java.io.BufferedReader; | ||
import java.io.BufferedWriter; | ||
import java.nio.file.Files; | ||
import java.nio.file.Path; | ||
import java.util.ArrayList; | ||
|
||
public class Storage { | ||
private final Path filePath; | ||
|
||
private void openFile() throws Exception { | ||
if (!Files.exists(this.filePath)) { | ||
Files.createDirectories(this.filePath.getParent()); | ||
Files.createFile(this.filePath); | ||
} | ||
} | ||
|
||
public Storage(Path filePath) { | ||
this.filePath = filePath; | ||
} | ||
|
||
public ArrayList<Task> loadData() throws Exception { | ||
ArrayList<Task> tasks = new ArrayList<Task>(); | ||
|
||
openFile(); | ||
BufferedReader reader = Files.newBufferedReader(this.filePath); | ||
int taskCount = Integer.parseInt(reader.readLine()); | ||
for(int i = 0; i < taskCount; ++i) { | ||
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. Should there be a white space character after 'for'? |
||
tasks.add(TaskParser.parseTask(reader)); | ||
} | ||
reader.close(); | ||
return tasks; | ||
} | ||
|
||
public void saveData(ArrayList<Task> tasks) throws Exception { | ||
openFile(); | ||
BufferedWriter writer = Files.newBufferedWriter(this.filePath); | ||
writer.write(String.valueOf(tasks.size())); | ||
writer.newLine(); | ||
for(Task task : tasks) { | ||
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. Should there be a white space character after 'for'? |
||
writer.write(task.toString()); | ||
writer.newLine(); | ||
} | ||
writer.close(); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,33 @@ | ||
public class Task { | ||
protected String description; | ||
private boolean isDone; | ||
|
||
public Task(String description) { | ||
this.description = description; | ||
this.isDone = false; | ||
} | ||
|
||
public Task(String description, boolean isDone) { | ||
this.description = description; | ||
this.isDone = isDone; | ||
} | ||
|
||
private String getStatusIcon() { | ||
return isDone ? "O" : "X"; | ||
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. Consider storing "O" and "X" as constants and then referring to them? |
||
} | ||
|
||
public boolean markAsDone() { | ||
if (this.isDone) { | ||
return false; | ||
} | ||
else { | ||
this.isDone = true; | ||
return true; | ||
} | ||
} | ||
|
||
@Override | ||
public String toString() { | ||
return String.format("[%s] %s", this.getStatusIcon(), this.description); | ||
} | ||
} | ||
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 feel that it would be better to add 1 empty line at the end of this file so that git knows that the file terminates here. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,33 @@ | ||
import java.util.ArrayList; | ||
|
||
public class TaskList { | ||
private ArrayList<Task> tasks; | ||
|
||
public TaskList() { | ||
this.tasks = new ArrayList<Task>(); | ||
} | ||
|
||
public TaskList(ArrayList<Task> tasks) { | ||
this.tasks = tasks; | ||
} | ||
|
||
public void addTask(Task newTask) { | ||
tasks.add(newTask); | ||
} | ||
|
||
public void removeTask(int position) { | ||
tasks.remove(position); | ||
} | ||
|
||
public ArrayList<Task> getTasks() { | ||
return this.tasks; | ||
} | ||
|
||
public int size() { | ||
return this.tasks.size(); | ||
} | ||
|
||
public Task get(int position) { | ||
return this.tasks.get(position); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,38 @@ | ||
import java.io.BufferedReader; | ||
|
||
public class TaskParser { | ||
public static Task parseTask(BufferedReader sc) throws Exception { | ||
String input = sc.readLine(); | ||
String taskType = input.substring(0, 3); | ||
String status = input.substring(3, 6); | ||
String rest = input.substring(7); | ||
boolean isDone = (status.equals("[O]")); | ||
Task result; | ||
|
||
switch (taskType) { | ||
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. Shouldn't there be no indentation for case clauses? |
||
case "[T]": { | ||
result = new Todo(rest, isDone); | ||
break; | ||
} | ||
case "[D]": { | ||
int position = rest.indexOf("(by: "); | ||
String description = rest.substring(0, position - 1); | ||
String by = rest.substring(position + 4, rest.length() - 1); | ||
result = new Deadline(description, isDone, by); | ||
break; | ||
} | ||
case "[E]": { | ||
int position = rest.indexOf("(at: "); | ||
String description = rest.substring(0, position - 1); | ||
String at = rest.substring(position + 4, rest.length() - 1); | ||
result = new Deadline(description, isDone, at); | ||
break; | ||
} | ||
default: { | ||
throw new DukeException("Error while loading data."); | ||
} | ||
} | ||
|
||
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. Is line 35 (blank statement) removable? |
||
return result; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,14 @@ | ||
public class Todo extends Task { | ||
public Todo(String description) { | ||
super(description); | ||
} | ||
|
||
public Todo(String description, boolean isDone) { | ||
super(description, isDone); | ||
} | ||
|
||
@Override | ||
public String toString() { | ||
return String.format("[T]%s", super.toString()); | ||
} | ||
} |
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 you can change "by" variable name to be more descriptive? Can consider "timeDescription"?