-
Notifications
You must be signed in to change notification settings - Fork 432
[Toh Zhen Yu, Nicholas] iP #457
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
base: master
Are you sure you want to change the base?
Changes from 30 commits
3b19ba1
d6b1456
49fdaf1
52ff3b3
48f8a1e
1c9189f
d91440c
28ca00a
b2c3edd
17f4128
f74aa3c
73ecf65
9d526f1
dea5581
f128e30
d3d481d
43fd410
eba4197
68f0af7
92603f8
54597fe
24f5318
9d2e2e4
a7ae636
8dfb179
99b108f
f143e67
3607d1a
316ab12
42e8830
3f2e05e
a75fcee
34f572c
d24d425
6de3764
9933b7b
6a8c20a
0177fda
0a4bb48
02bc264
7671eb9
dc66c72
aedb323
41238c8
9132476
1467d91
7b29628
29d63f0
d7274aa
98466a8
16f9125
4fd75cc
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 |
|---|---|---|
|
|
@@ -15,3 +15,5 @@ bin/ | |
|
|
||
| /text-ui-test/ACTUAL.txt | ||
| text-ui-test/EXPECTED-UNIX.TXT | ||
|
|
||
| *.ser | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,23 @@ | ||
| import java.time.LocalDate; | ||
|
|
||
| /** | ||
| * Represents a task to be completed by a certain time | ||
| */ | ||
| public class Deadline extends Task { | ||
|
|
||
| /** | ||
| * @param description Task 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. There should be a punctuation behind every parameter description. I have noticed similar issue in other places too. |
||
| * @param date Deadline for the task | ||
| */ | ||
| public Deadline(String description, LocalDate date) { | ||
| super(description, date); | ||
| } | ||
|
|
||
| /** | ||
| * @return String representation of the task | ||
| */ | ||
| @Override | ||
| public String toString() { | ||
| return "[D]" + super.toString() + " (by: " + date.toString() + ")\n"; | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,10 +1,34 @@ | ||
| /** | ||
|
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. Perhaps putting the class in a package? |
||
| * Main class for Duke program. | ||
| */ | ||
| public class Duke { | ||
| public static void main(String[] args) { | ||
| String logo = " ____ _ \n" | ||
| + "| _ \\ _ _| | _____ \n" | ||
| + "| | | | | | | |/ / _ \\\n" | ||
| + "| |_| | |_| | < __/\n" | ||
| + "|____/ \\__,_|_|\\_\\___|\n"; | ||
| System.out.println("Hello from\n" + logo); | ||
|
|
||
| /** | ||
| * Entry point of the duke program. Read from storage and creates a UI class instance. | ||
| * Serves as intermediate between the UI and parser. | ||
|
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 is incorrect as there should be an empty line between the description and parameter. I have noticed similar issues in several other places too. |
||
| * @param args | ||
| * @throws DukeException | ||
| */ | ||
| public static void main(String[] args) throws DukeException { | ||
| TaskList tasks = Storage.read(); | ||
| UI ui = new UI(); | ||
| ui.welcome(tasks); | ||
| String input = ui.getInput(); | ||
| Parser parser = new Parser(tasks); | ||
| while (!input.equals("bye")) { | ||
| try { | ||
| if (input.isEmpty()) { | ||
| input = ui.getInput(); | ||
| continue; | ||
| } | ||
| parser.parse(input); | ||
| } catch (DukeException e) { | ||
| UI.print(e.getMessage() + "\n"); | ||
| } | ||
|
|
||
| input = ui.getInput(); | ||
| } | ||
| ui.bye(); | ||
| } | ||
|
|
||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,12 @@ | ||
| /** | ||
| * An exception class used for errors caused by inappropriate user input. Displays a message. | ||
| */ | ||
| public class DukeException extends Exception { | ||
| /** | ||
| * Constructor for DukeException. | ||
| * @param msg the message to be displayed. | ||
| */ | ||
| DukeException(String msg) { | ||
| super("☹ OOPS!!! " + msg); | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,23 @@ | ||
| import java.time.LocalDate; | ||
|
|
||
| /** | ||
| * Represents a task occurring at a certain time | ||
| */ | ||
| public class Event extends Task { | ||
|
|
||
| /** | ||
| * @param description Task description | ||
| * @param date Date of the event | ||
| */ | ||
| public Event(String description, LocalDate date) { | ||
| super(description, date); | ||
| } | ||
|
|
||
| /** | ||
| * @return String representation of the task | ||
| */ | ||
| @Override | ||
| public String toString() { | ||
| return "[E]" + super.toString() + " (at: " + date.toString() + ")\n"; | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| Manifest-Version: 1.0 | ||
| Main-Class: Duke | ||
|
|
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,95 @@ | ||
| import java.time.LocalDate; | ||
| import java.time.format.DateTimeParseException; | ||
|
|
||
| /** | ||
| * Class that handles user input | ||
| */ | ||
| public class Parser { | ||
| TaskList tasks; | ||
|
|
||
| /** | ||
| * @param tasks TaskList to use | ||
| */ | ||
| public Parser(TaskList tasks) { | ||
| this.tasks = tasks; | ||
| } | ||
|
|
||
| /** | ||
| * Parses the user command | ||
| * @param input input from the user | ||
| * @throws DukeException | ||
| */ | ||
| void parse(String input) throws DukeException { | ||
| String[] words = input.split(" "); | ||
| String command = words[0]; | ||
| Task newTask; | ||
| String[] subst; | ||
| LocalDate date; | ||
| switch (command) { | ||
| case "list": | ||
|
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 is incorrect because there should not be any indentation for the case clauses. |
||
| tasks.print_tasks(); | ||
| break; | ||
| case "done": | ||
| case "delete": | ||
|
Comment on lines
+40
to
+41
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. The "done" case should end with a break; statement. You might have unintentionally shifted the "done" case into the "delete" case.
Author
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. Hi, this was intentional as both commands use the same logic of tokenizing the input to get the task number, then getting that task object. It is after these steps where the logic then differs. Do you have a suggestion of a better way to do this? |
||
| int i = Integer.parseInt(words[1]) - 1; | ||
| Task task = tasks.get(i); | ||
| if (command.equals("done")) { | ||
| tasks.setDone(i, true); | ||
| UI.print("Nice! I've marked this task as done: \n" + task); | ||
| } else if (command.equals("delete")) { | ||
| tasks.remove(i); | ||
| UI.print("Noted. I've removed this task: \n" + task + tasks.numTasks()); | ||
| } | ||
| break; | ||
| case "todo": | ||
| if (input.length() < 6) { | ||
| throw new DukeException("The description of a todo cannot be empty."); | ||
| } | ||
| String text = input.substring(5); | ||
| newTask = new Todo(text); | ||
| tasks.addTask(newTask); | ||
| break; | ||
| case "deadline": | ||
| if (input.length() < 10) { | ||
| throw new DukeException("The description of a deadline cannot be empty."); | ||
| } | ||
| subst = input.substring(9).split(" /by "); | ||
| if (subst.length < 2) { | ||
| throw new DukeException("The due date must be specified."); | ||
| } | ||
| try { | ||
| date = LocalDate.parse(subst[1]); | ||
| newTask = new Deadline(subst[0], date); | ||
| tasks.addTask(newTask); | ||
| } catch (DateTimeParseException e) { | ||
| throw new DukeException(e.getMessage()); | ||
| } | ||
| break; | ||
| case "event": | ||
| if (input.length() < 7) { | ||
| throw new DukeException("The description of an event cannot be empty."); | ||
| } | ||
| subst = input.substring(6).split(" /at "); | ||
| if (subst.length < 2) { | ||
| throw new DukeException("The event date must be specified."); | ||
| } | ||
| try { | ||
| date = LocalDate.parse(subst[1]); | ||
| newTask = new Event(subst[0], date); | ||
| tasks.addTask(newTask); | ||
| } catch (DateTimeParseException e) { | ||
| throw new DukeException(e.getMessage()); | ||
| } | ||
| break; | ||
| case "find": | ||
| if (input.length() < 6) { | ||
| throw new DukeException("Text to search for cannot be empty."); | ||
| } | ||
| tasks.find(input.substring(5)); | ||
| break; | ||
| default: | ||
| throw new DukeException("I'm sorry, but I don't know what that means :-("); | ||
| // break; | ||
|
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. A break statement should be included for the default case too.
Author
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. Hmm intellij complains about this as it's an unreachable statement. Do you know how I can configure it to ignore this error, or have a suggestion on how to implement this logic instead? |
||
| } | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,55 @@ | ||
| import java.io.*; | ||
|
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. The imported classes should be listed explicitly. |
||
| import java.nio.file.Paths; | ||
|
|
||
| /** | ||
| * Class that provides file storage utilities | ||
| */ | ||
| public class Storage { | ||
| public static final File storage_file = Paths.get("tasks.ser").toFile(); | ||
|
|
||
| public Storage() { | ||
| } | ||
|
|
||
| /** | ||
| * Serializes a TaskList and writes it to file | ||
| * @param t tasklist to be stored | ||
| */ | ||
| public static void store(TaskList t) { | ||
| try { | ||
| //noinspection ResultOfMethodCallIgnored | ||
| storage_file.createNewFile(); //creates file if it does not exist | ||
| FileOutputStream fileOut = new FileOutputStream(storage_file); | ||
| ObjectOutputStream out = new ObjectOutputStream(fileOut); | ||
| out.writeObject(t); | ||
| out.close(); | ||
| } catch (IOException e) { | ||
| e.printStackTrace(); | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Reads a serialized TaskList | ||
| * @return tasklisk read from file | ||
| */ | ||
| public static TaskList read() { | ||
| try { | ||
| TaskList t; | ||
| FileInputStream fileIn = new FileInputStream(storage_file); | ||
| ObjectInputStream in = new ObjectInputStream(fileIn); | ||
| t = (TaskList) in.readObject(); | ||
| in.close(); | ||
| fileIn.close(); | ||
| t.loadMessage = "Successfully loaded from storage. " + t.numTasks(); | ||
| return t; | ||
| } catch (FileNotFoundException i) { | ||
| return new TaskList(); | ||
| } catch (IOException i) { | ||
| i.printStackTrace(); | ||
| return new TaskList(); | ||
| } catch (ClassNotFoundException c) { | ||
| System.out.println("Employee class not found"); | ||
| c.printStackTrace(); | ||
| return new TaskList(); | ||
| } | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,26 @@ | ||
| import java.time.LocalDate; | ||
|
|
||
| /** | ||
| * Represents a task | ||
| */ | ||
| public class Task implements java.io.Serializable { | ||
| String text; | ||
| boolean done; | ||
|
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. Perhaps a more intuitive variable name that will sound more like a boolean? |
||
| LocalDate date; | ||
|
|
||
| /** | ||
| * @param text Task description | ||
| * @param date Date of the task | ||
| */ | ||
| public Task(String text, LocalDate date) { | ||
| this.text = text; | ||
| this.date = date; | ||
| } | ||
|
|
||
| /** | ||
| * @return String representation of the task | ||
| */ | ||
| public String toString() { | ||
| return "[" + (done ? "✓" : "✗") + "] " + text; | ||
| } | ||
| } | ||
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 adding a short summary of the method? This can also apply to other methods that do not have a description in other places.
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.
thanks, not sure how I should describe the constructor or toString, do you have suggestions?