[Low Xi Zhi] iP by lowxizhi · Pull Request #55 · nus-cs2113-AY1920S2/duke

lowxizhi · 2020-01-31T08:53:20Z

No description provided.

A11riseforme

a good effort overall! good job

A11riseforme · 2020-02-04T05:24:38Z

+                switch (taskType){
+                    case "todo":
+                        t = new Todo(words[1]);
+                        break;
+                    case "deadline":
+                        String[] deadlineWords = words[1].split(" /by ", 2); // split the deadline task from the by string using /by as the delimiter
+                        t = new Deadline(deadlineWords[0], deadlineWords[1]);
+                        break;
+                    case "event":
+                        String[] eventWords = words[1].split(" /at ", 2); // split the event task from the by string using /at as the delimiter
+                        t = new Event(eventWords[0], eventWords[1]);
+                        break;


Indentation is violating the java coding standard. There should be no indentation for case clauses. follow the style below:

switch (condition) { case ABC: statements; // Fallthrough case DEF: statements; break; case XYZ: statements; break; default: statements; break; }

A11riseforme · 2020-02-04T05:39:45Z

+    }
+
+    private static void printListMessage(Task[] taskList, int taskCount) {
+        System.out.println("\t____________________________________________________________\n"


The string literal can be isolated as a vairable like lineDivider

private static final String LINE_DIVIDER= "\t____________________________________________________________\n";

can call this viriable in the later codes.

A11riseforme · 2020-02-04T05:52:00Z

+            String[] words = line.split(" ",2); // split the first word from the rest of the sentence using space as the delimiter
+            if (line.equals("list")) {
+                printListMessage(taskList, taskCount);
+            } else if (words[0].equals("done")) {
+                int taskNum = Integer.parseInt(line.substring(5,6));
+                Task t=taskList[taskNum-1];
+                t.markAsDone();
+                printDoneMessage(t);
+            } else {
+                String taskType = words[0];
+                Task t = new Task(line);
+                switch (taskType){
+                    case "todo":
+                        t = new Todo(words[1]);
+                        break;
+                    case "deadline":
+                        String[] deadlineWords = words[1].split(" /by ", 2); // split the deadline task from the by string using /by as the delimiter
+                        t = new Deadline(deadlineWords[0], deadlineWords[1]);
+                        break;
+                    case "event":
+                        String[] eventWords = words[1].split(" /at ", 2); // split the event task from the by string using /at as the delimiter
+                        t = new Event(eventWords[0], eventWords[1]);
+                        break;
+                }
+                taskList[taskCount] = t;
+                taskCount++;
+
+                printAddTaskMessage(taskCount, t);
+            }


This can be combined into one single switch case structure. extract the first word of the user input.

switch (command) { case "list": // list logic case "done": // done logic case "todo": // todo logic case "deadline": . . . }

jiajuinphoon

Overall, you followed the coding standard except the switch case, and the code is generally neat and comfortable to read. Great job. All the best for your ip :)
Also took part in it : @trishaangelica

jiajuinphoon · 2020-02-04T05:35:32Z

+                String taskType = words[0];
+                Task t = new Task(line);
+                switch (taskType){
+                    case "todo":


Good job for using switch, will recommend to follow the coding standard because the word "case" must by directly under the switch :)

jiajuinphoon · 2020-02-04T05:38:06Z

+
+        while (!line.equals("bye")){
+            String[] words = line.split(" ",2); // split the first word from the rest of the sentence using space as the delimiter
+            if (line.equals("list")) {


good job to follow the coding standard :)

jiajuinphoon · 2020-02-04T05:44:34Z

+        printGoodbyeMessage();
+    }
+
+    private static void printListMessage(Task[] taskList, int taskCount) {


This way of printing the task seems neater compared to how I did it. Good Job :)

jiajuinphoon · 2020-02-04T05:49:37Z

+                + "\t____________________________________________________________\n");
+    }
+
+    private static void printGoodbyeMessage() {


Good job by creating several functions instead of putting everything in the main. :)

Shannonwje

Overall amazing work, especially in explaining the lines of code that may seem confusing to the code-reader. But overall tidiness and javadoc comments can be even better

Shannonwje · 2020-02-04T05:29:05Z

                + "| |_| | |_| |   <  __/\n"
                + "|____/ \\__,_|_|\\_\\___|\n";
        System.out.println("Hello from\n" + logo);
+        System.out.println("\t____________________________________________________________\n"


Maybe you can consider the printing of the welcome message in another method for the code to be tidier, like what you did for the goodbye message. -SWJE

Shannonwje · 2020-02-04T05:34:54Z

+        String line = in.nextLine();
+
+        while (!line.equals("bye")){
+            String[] words = line.split(" ",2); // split the first word from the rest of the sentence using space as the delimiter


Line 25 of the code seems very long, maybe can consider placing the comment above line 25 to make the code tidier. But good job on explaining the the use of the function in your code. -SWJE

Shannonwje · 2020-02-04T05:38:23Z

+                printDoneMessage(t);
+            } else {
+                String taskType = words[0];
+                Task t = new Task(line);


Maybe consider having a more meaningful variable name to make the code more readable such as newTask. -SWJE

Shannonwje · 2020-02-04T05:41:32Z

+                String taskType = words[0];
+                Task t = new Task(line);
+                switch (taskType){
+                    case "todo":


Can reference the java coding standard for CS2113T for the switch-case statement indentations. There is no indentation for case clauses, for eg.

switch (condition) {
case ABC:
statements;
// Fallthrough
case DEF:
statements;
break;
case XYZ:
statements;
break;
default:
statements;
break;
}

SWJE

Shannonwje · 2020-02-04T05:45:52Z

+        printGoodbyeMessage();
+    }
+
+    private static void printListMessage(Task[] taskList, int taskCount) {


Maybe can consider adding in javadoc comments for methods inside the class. But otherwise, amazing work on making your work tidier by placing them into different methods. -SWJE

Shannonwje · 2020-02-04T05:48:54Z

+        this.at = at;
+    }
+
+//    public void setAt(String at) {


Maybe can delete away methods that are not needed or used so that the code looks tidier. But awesome code tidiness!!! - SWJE

…line and event exceptions

…b-package. put all classes in the duke package. add exception for commands with only whitespace.

Level-9 add find feature

lowxizhi added 7 commits January 23, 2020 16:04

add welcome message

7128136

add echo function and exit message

6bdcfa4

add ability to store and list items entered by the user

945c175

add mark as done function

6af769b

Tweak code to comply with coding standard

71d7db8

added todo, event, and deadline tasks

e552f3e

refactor print messages

6e5912f

A11riseforme reviewed Feb 4, 2020

View reviewed changes

jiajuinphoon reviewed Feb 4, 2020

View reviewed changes

Shannonwje reviewed Feb 4, 2020

View reviewed changes

lowxizhi and others added 20 commits February 11, 2020 09:02

handle errors with exceptions

332f001

handle errors with exceptions

1f18dac

add message in printUnknownWordExceptionMessage

11e048e

Merge branch 'Level-5'

7f50899

add delete command, store tasks in ArrayList and corrected empty dead…

885234a

…line and event exceptions

add duke.txt file to load and write tasks

a213839

Merge branch 'Level-6'

df58138

merge Level-7 and Level-6 changes

612c36f

correct path for creating duke.txt

a6aafab

extract UI, Storage, Parser, TaskList classes. put exceptions in a su…

993475f

…b-package. put all classes in the duke package. add exception for commands with only whitespace.

Level-9 add find feature

f4ba786

A-JavaDoc: add JavaDoc comments

fbeac71

Merge pull request #2 from lowxizhi/branch-Level-9

ee29488

Level-9 add find feature

merge master to

48377ba

Merge branch 'A-JavaDoc'

f4ba581

handle NumberFormatException

3314af1

add user guide in docs/README.md and updated jar

7857100

Set theme jekyll-theme-cayman

f340742

Update README.md

2464073

Update README.md

6b98f15

lowxizhi added 2 commits March 1, 2020 23:46

Set theme jekyll-theme-cayman

fc36a1b

Update README.md

a7713ee

Uh oh!

Conversation

lowxizhi commented Jan 31, 2020

Uh oh!

A11riseforme left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jiajuinphoon left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Shannonwje left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants