[Zhao Yuqing] iP by z0723-julie · Pull Request #67 · nus-cs2113-AY2122S2/ip

z0723-julie · 2022-02-02T03:45:44Z

No description provided.

shxr3f

Well written code based on Coding Statndards.
Got to take a look at switch statements.

shxr3f · 2022-02-03T01:48:06Z

+    private static void showList() {
+        System.out.println("    ____________________________________________________________");
+        System.out.println("     Here are the tasks in your list:");
+        for (int i = 0; i < descriptions.size(); i++) {


Spacing is always present in loops and conditional statements.

shxr3f · 2022-02-03T01:48:10Z

+            switch (type) {
+                case "T":
+                    Todo todo = new Todo(description);
+                    System.out.println(todo.toString(isDone));
+                    break;
+                case "D":
+                    Deadline deadline = new Deadline(description, date);
+                    System.out.println(deadline.toString(isDone));
+                    break;
+                case "E":
+                    Event event = new Event(description, date);
+                    System.out.println(event.toString(isDone));
+                    break;
+                default:
+                    System.out.println("       Unknown Type");
+            }


No indentation for case clauses under switch statements

Thank you for pointing it out, I will work on it!

shxr3f · 2022-02-03T01:48:12Z

+        switch (type) {
+            case "T":
+                Todo todo = new Todo(description);
+                System.out.println("       " + todo.toString(true));
+                break;
+            case "D":
+                Deadline deadline = new Deadline(description, date);
+                System.out.println("       " + deadline.toString(true));
+                break;
+            case "E":
+                Event event = new Event(description, date);
+                System.out.println("       " + event.toString(true));
+                break;
+            default:
+                System.out.println("       Unknown Type");
+        }


No indentation for case clauses under switch statements

Thank you for pointing it out, I will work on it!

shxr3f · 2022-02-03T01:48:20Z

+    private static ArrayList<String> descriptions = new ArrayList<>();
+    private static ArrayList<Boolean> dones = new ArrayList<>();
+    private static ArrayList<String> types = new ArrayList<>();
+    private static ArrayList<String> dates = new ArrayList<>();
+


Following plural form for Arrays! Good naming conventions used throughout.

lelerer · 2022-02-03T02:38:45Z

-        System.out.println("____________________________________________________________");
-        System.out.println(exits);
-        System.out.println("____________________________________________________________");
+        greeting();


Good modularization here.

lelerer · 2022-02-03T02:43:29Z

@@ -0,0 +1,14 @@
+public class Deadline extends Task {
+
+    protected String by;


The variable name might not be meaningful enough, maybe change to "deadline"

I will reconsider that.

lelerer · 2022-02-03T02:46:25Z

-}
+
+    private static void echo() {
+        String line;


The meaning of “line” is a bit unclear, maybe consider change it into "input"

I will reconsider that.

lelerer · 2022-02-03T02:49:02Z

-        System.out.println("    ____________________________________________________________");
+        System.out.println("     Here are the tasks in your list:");
+        for (int i = 0; i < descriptions.size(); i++) {
+            System.out.println(i+1 + "." + descriptions.get(i));


There should always be spaces before and after +

Thank you for pointing it out, I will work on it!

lelerer · 2022-02-03T02:50:27Z

+                int number = Integer.parseInt(line.substring(line.length() - 1));
+                markAsDone(number);
+                line = in.nextLine();
+                continue;


The continue here is redundant

Thank you for pointing it out, I will work on it!

lelerer · 2022-02-03T02:54:41Z

@@ -0,0 +1,14 @@
+public class Deadline extends Task {
+
+    protected String by;


The variable name might not be meaningful enough, maybe change to "deadline"

i will consider it!

lelerer · 2022-02-03T02:54:55Z

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


Good modularization here.

lelerer · 2022-02-03T02:55:08Z

-}
+
+    private static void getCommand() {
+        String line;


The meaning of “line” is a bit unclear, maybe consider change it into "input"

I will consider that.

lelerer · 2022-02-03T02:55:20Z

+                int number = Integer.parseInt(line.substring(line.length() - 1));
+                markAsDone(number);
+                line = in.nextLine();
+                continue;


The continue here is redundant.

I will consider that.

lelerer · 2022-02-03T02:56:33Z

@@ -0,0 +1,14 @@
+public class Event extends Task {
+
+    protected String at;


You might want to reconsider the variable name "at" also, it's unclear about the meaning

I will consider that.

lelerer · 2022-02-03T02:57:42Z

+                    Todo todo = new Todo(description);
+                    System.out.println(todo.toString(isDone));
+                    break;
+                case "D":


No indentation required for case statement

Thanks for pointing it out, I will fix it.

warrencxw

Code was generally well done. There are just a few potential areas for improvement to help readability and code quality to consider.

warrencxw · 2022-02-03T02:54:41Z

        String logo = " ____        _        \n"
                + "|  _ \\ _   _| | _____ \n"
                + "| | | | | | | |/ / _ \\\n"
                + "| |_| | |_| |   <  __/\n"
                + "|____/ \\__,_|_|\\_\\___|\n";


Consider making this logo a constant instead, as it would not be changed.

I will reconsider that.

warrencxw · 2022-02-03T02:56:59Z

+        System.out.println("    ____________________________________________________________");
+        System.out.println("     Bye. Hope to see you again soon!");
+        System.out.println("    ____________________________________________________________");


Consider making the String here a constant with perhaps a name of FAREWELL_MESSAGE, so that it is clear that you are printing a closing message at the end.

I will reconsider that.

warrencxw · 2022-02-03T03:00:47Z

+    private static void markAsDone(int number) {
+        System.out.println("    ____________________________________________________________");
+        System.out.println("     Nice! I've marked this task as done:");
+        int index = number - 1;


Consider naming the variable as something that may be more easily understood at one glance, e.g. taskNumber, taskIndex

Thank you for pointing it out, I will work on it!

warrencxw · 2022-02-03T03:08:16Z

+        System.out.println("    ____________________________________________________________");
+    }
+
+    private static void greeting() {


Consider renaming greeting to a verb to make it clearer what this method does.

Thank you for pointing it out, I will work on it!

warrencxw · 2022-02-03T03:12:41Z

+        dones.add(false);
+        if (line.startsWith("todo")) {
+            types.add("T");
+            description = line.substring(5);


Consider extracting out this magic literal '5' into a constant to make it clearer why this substring is made.

Thank you for pointing it out, I will work on it!

laiisaac

There are some good practices overall, but I think using more Single Level Abstraction could make the code more readable

laiisaac · 2022-02-03T03:17:11Z

+        Scanner in = new Scanner(System.in);
+        // get user input
+        line = in.nextLine();


Maybe a method of getUserInput would increase readability?

laiisaac · 2022-02-03T03:27:42Z

+        System.out.println("    ____________________________________________________________");
+    }
+
+    private static void handleCommand(String line) {


handleCommand method looks a little long, perhaps adding methods for adding different events can increase readability?

laiisaac · 2022-02-03T03:31:05Z

+                line = in.nextLine();
+                continue;
+            }
+            // Command is valid, handle the command


I like the occasional comments which clarify parts of the code

laiisaac · 2022-02-03T03:33:47Z

+        }
+    }
+
+    private static void showList() {


I like methods like this where the name makes the intention obvious, would love to see more of such methods.

swongts · 2022-03-01T15:20:33Z

+                continue;
+            }
+            if (input.startsWith("done")) {
+                int number = Integer.parseInt(input.substring(input.length() - 1));


Create a separate variable for input.substring(input.length() - 1) to make the code more readable

swongts · 2022-03-01T15:21:01Z

+                continue;
+            }
+            if (input.startsWith("delete")) {
+                int number = Integer.parseInt(input.substring(input.length() - 1));


Do the parseInt in deleteTask function to maintain SLAP

swongts · 2022-03-01T15:22:22Z

+        }
+    }
+
+    private static void loadTasks() {


You can create a separate class for things that have to do with the storing of information into a text file, instead of including it all in Duke

swongts · 2022-03-01T15:22:59Z

+
+    private static void markAsDone(int number) {
+        System.out.println("    ____________________________________________________________");
+        System.out.println("     Nice! I've marked this task as done:");


Create constants for these messages

swongts · 2022-03-01T15:24:20Z

+        System.out.println("    ____________________________________________________________");
+        System.out.println("     Nice! I've marked this task as done:");
+        int index = number - 1;
+        String description = tasks.get(index);


Can consider creating an exception for if the task with that index doesnt exist. E.g. if your tasks length is only 2 and someone inputs 5 as the index

Add Find method

Add JavaDoc comments

z0723-julie added 7 commits January 20, 2022 13:46

Add greetings and exits

1c21833

Add greet, echo, exit

c16d014

Add list

980b88f

Mark as done

b262d0a

Update code to match coding standards

fdf049c

Add ToDos, Events, Deadlines

5aaf72b

Improve code quality

8c9efc1

shxr3f reviewed Feb 3, 2022

View reviewed changes

lelerer reviewed Feb 3, 2022

View reviewed changes

warrencxw reviewed Feb 3, 2022

View reviewed changes

laiisaac reviewed Feb 3, 2022

View reviewed changes

z0723-julie added 14 commits February 3, 2022 12:03

Automate Text UI Testing

b92a207

Add exception classes

b58daed

Handle errors

dac86dc

Fix coding quality

ec013a0

Merge branch 'branch-Level-5'

cd7ac6c

Organize into packages

a0871a9

Add exceptions specific to Duke

d1dfe47

Add support for deleting tasks from the list

5d2fbf5

Use ArrayList for storing data

2be2af4

Save tasks to file

f24d79c

Merge branch 'branch-level-6'

6fefd9d

Edit duke.txt

bc19982

Merge branch 'branch-level-7'

8b723d6

Add manifest

ff816ca

Add read file function

a3e8263

swongts reviewed Mar 1, 2022

View reviewed changes

z0723-julie and others added 11 commits March 2, 2022 20:55

Update README.md

c04fe7e

Add a User Guide

2b52659

Add Find method

5ed41eb

Merge pull request #1 from z0723-julie/branch-Level-9

64603b7

Add Find method

Add JavaDoc comments

866a688

Merge pull request #2 from z0723-julie/A-JavaDoc

1521d62

Add JavaDoc comments

Use More OOP

bb6507d

Modify error

8a9dcb1

Update text-ui-test

4d18add

Adjust according to peer evaluation

984d996

Update Error handling

dae3979

		@@ -0,0 +1,14 @@
		public class Deadline extends Task {

		protected String by;

		@@ -0,0 +1,14 @@
		public class Event extends Task {

		protected String at;

Uh oh!

Conversation

z0723-julie commented Feb 2, 2022

Uh oh!

shxr3f 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!

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!

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!

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!

warrencxw 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!

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!