[ngyongjian] iP #208

ngyongjian · 2023-02-01T13:29:25Z

No description provided.

raydent30

Good job on your code!
Could use some reformating.

raydent30 · 2023-02-03T02:59:18Z

src/main/java/Duke.java

+                if (input.equals("list")) {
+                    output = "Here are the tasks in the list:" + System.lineSeparator() + Task.printTasksList(tasks);
+
+                } else if (input.matches("mark [0-9]{1,2}")) {
+                    String[] marks = input.split(" ");
+                    tasks.get(Integer.parseInt(marks[1]) - 1).markAsDone();
+                    output = "Nice! I've marked this task as done:" + System.lineSeparator()
+                            + tasks.get(Integer.parseInt(marks[1]) - 1).toString();
+                } else if (input.matches("unmark [0-9]{1,2}")) {
+                    String[] marks = input.split(" ");
+                    tasks.get(Integer.parseInt(marks[1]) - 1).markAsNotDone();
+                    output = "OK, I've marked this task as not done yet:" + System.lineSeparator()
+                            + tasks.get(Integer.parseInt(marks[1]) - 1).toString();
+                } else if (input.startsWith("todo", 0)) {
+                    String toDoDesc = input.split("todo")[1].trim();
+                    Task toDo = new Todo(toDoDesc);
+                    tasks.add(toDo);
+                    output = "Got it. I've added this task:" + System.lineSeparator()
+                            + toDo.toString() + System.lineSeparator() + "Now you have " + Task.numberOfTasks
+                            + " in the list.";
+                } else if (input.startsWith("deadline", 0)) {
+                    String deadlineDesc = input.split("/")[0].split("deadline")[1].trim();
+                    String deadlineDay = input.split("/")[1].trim();
+                    Task deadline = new Deadline(deadlineDesc, deadlineDay);
+                    tasks.add(deadline);
+                    output = "Got it. I've added this task:" + System.lineSeparator()
+                            + deadline.toString() + System.lineSeparator() + "Now you have " + Task.numberOfTasks
+                            + " in the list.";
+                } else if (input.startsWith("event", 0)) {
+                    String eventDesc = input.split("/")[0].split("event")[1].trim();
+                    String start = input.split("/")[1].trim();
+                    String end = input.split("/")[2].trim();
+                    Task event = new Event(eventDesc, start, end);
+                    tasks.add(event);
+                    output = "Got it. I've added this task:" + System.lineSeparator()
+                            + event.toString() + System.lineSeparator() + "Now you have " + Task.numberOfTasks
+                            + " in the list.";
+                } else {
+                    Task task = new Task(input);
+                    tasks.add(task);
+                    output = "Got it. I've added this task:" + System.lineSeparator()
+                            + task.toString() + System.lineSeparator() + "Now you have " + Task.numberOfTasks
+                            + " in the list.";
+                }


Good code readability with the else-if statements!

raydent30 · 2023-02-03T03:01:20Z

src/main/java/Duke.java

+        System.out.println("What can I do for you?\n");
+        ArrayList<Task> tasks = new ArrayList<Task>();
+        try (Scanner scan = new Scanner(System.in)) {
+            String input = scan.nextLine();


"Arrow Head" code, maybe you can consider avoiding deep nesting?

raydent30 · 2023-02-03T03:02:19Z

src/main/java/Duke.java

+                            + event.toString() + System.lineSeparator() + "Now you have " + Task.numberOfTasks
+                            + " in the list.";
+                } else {
+                    Task task = new Task(input);


A lot of complex strings/formulas, maybe consider splitting them up?

raydent30 · 2023-02-03T03:04:43Z

src/main/java/Duke.java

+                    Task event = new Event(eventDesc, start, end);
+                    tasks.add(event);
+                    output = "Got it. I've added this task:" + System.lineSeparator()
+                            + event.toString() + System.lineSeparator() + "Now you have " + Task.numberOfTasks


A lot of "magic numbers" involved. Maybe can consider using constants instead?

Try not to use magic numbers. Can consider using an aptly named variable.

Can consider using switch case instead of if else to decide what to do based on the first word entered

YC-Michael

Overall, good naming and layout.

YC-Michael · 2023-02-03T02:43:55Z

src/main/java/Duke.java

@@ -6,5 +9,68 @@ public static void main(String[] args) {
                + "| |_| | |_| |   <  __/\n"
                + "|____/ \\__,_|_|\\_\\___|\n";
        System.out.println("Hello from\n" + logo);
+        System.out.println("Hello! I'm Duke");
+        System.out.println("What can I do for you?\n");
+        ArrayList<Task> tasks = new ArrayList<Task>();


I like how you used a resizable array to store the tasks instead of a fixed size of 100.

YC-Michael · 2023-02-03T04:22:14Z

src/main/java/Duke.java

+        System.out.println("Hello! I'm Duke");
+        System.out.println("What can I do for you?\n");
+        ArrayList<Task> tasks = new ArrayList<Task>();
+        try (Scanner scan = new Scanner(System.in)) {


Please follow the Java coding standards when using exceptions.

Suggested change

try (Scanner scan = new Scanner(System.in)) {

try {

Scanner scan = new Scanner(System.in)

YC-Michael · 2023-02-03T04:29:59Z

src/main/java/Duke.java

+                System.out.println("___________________________________________________");
+                input = scan.nextLine();
+            }
+        } catch (NumberFormatException e) {


Good use of exceptions here

YC-Michael · 2023-02-03T04:46:30Z

src/main/java/Event.java

@@ -0,0 +1,15 @@
+public class Event extends Task {
+    protected String start;


variables such as start and end can be more descriptive.

theopin

Good implementation on Duke so far! Some general points to improve your code:

Use of magic literals: Do try to identify such literals and replace them with named constants so that readers can better understand what your code aims to achieve
Long Main method: Due to implementation of all the main features of duke being consolidated in the main function, it seems difficult to read and understand the flow of it. Do consider refacotring these features into seperate methods to enhance understandability of the code

theopin · 2023-02-16T02:07:42Z

src/main/java/Duke.java

+                    } else if (input.startsWith("mark")) {
+                        String[] marks = input.split(" ");
+                        tasks.get(Integer.parseInt(marks[1]) - 1).markAsDone();
+                        output = "Nice! I've marked this task as done:" + System.lineSeparator()
+                                + tasks.get(Integer.parseInt(marks[1]) - 1).toString();


Try extracting such chunks into a seperate method to ease readability of the main function as it seems to be too big currently

theopin · 2023-02-16T02:09:09Z

src/main/java/Duke.java

+                        String deadlineDesc = input.split("/")[0].split("deadline")[1].trim();
+                        String deadlineDay = input.split("/")[1].trim();


Strings such as "/" and "deadline" seem to suggest usage of magic literals, do consider declaring them as named constants to enhance readability and understandability

theopin · 2023-02-16T02:12:35Z

src/main/java/Duke.java

+                    } else if (input.startsWith("event", 0)) {
+                        String eventDesc = input.split("/")[0].split("event")[1].trim();
+                        String start = input.split("/")[1].trim();


Indentation seems off here, should be 4 instead of 2

Branch level 9

add JavaDoc

ngyongjian added 8 commits January 27, 2023 10:31

Level 1. Greet, Echo, Exit

fff13bd

Level 1. Greet, Echo, Exit

d5c0add

Level 2. Add, List

900b481

Level 2

6b160da

Level 3. Mark as Done

ef4b3fb

Level 4. ToDos, Events, Deadlines

443d336

A-CodingStandard

49f3394

A-CodeQuality

03e770e

raydent30 reviewed Feb 3, 2023

View reviewed changes

YC-Michael reviewed Feb 3, 2023

View reviewed changes

ngyongjian added 5 commits February 8, 2023 00:51

A-Exception

c010346

Level 5. Handle Errors

caa5969

Organize into packages

06d3da3

Level 6

161d9d3

A-Collections

1672649

theopin reviewed Feb 16, 2023

View reviewed changes

ngyongjian and others added 14 commits February 19, 2023 11:10

Level-7

8926248

undo Level-7

2657473

branch-Level-7

a31c6c2

Level-7

9f407e0

NumberOfTask debug

cd97e4c

A-MoreOOP

a178113

Level-9

2c054d4

Merge pull request #1 from ngyongjian/branch-Level-9

713b40a

Branch level 9

add JavaDoc

10af1c0

Merge pull request #2 from ngyongjian/A-JavaDoc

8aabdd8

add JavaDoc

Update README.md

f2ad465

Update README.md

ddc4626

Update README.md

30670ce

Update README.md

478a610

ngyongjian added 2 commits March 4, 2023 15:50

Update README.md

d35fc8f

Update README.md

f8ea312

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[ngyongjian] iP #208

[ngyongjian] iP #208

ngyongjian commented Feb 1, 2023

raydent30 left a comment

raydent30 Feb 3, 2023

raydent30 Feb 3, 2023

raydent30 Feb 3, 2023

raydent30 Feb 3, 2023

YatPang Feb 3, 2023

YatPang Feb 3, 2023

YC-Michael left a comment

YC-Michael Feb 3, 2023

YC-Michael Feb 3, 2023

YC-Michael Feb 3, 2023

YC-Michael Feb 3, 2023

theopin left a comment

theopin Feb 16, 2023

theopin Feb 16, 2023

theopin Feb 16, 2023

	try (Scanner scan = new Scanner(System.in)) {
	try {
	Scanner scan = new Scanner(System.in)

		@@ -0,0 +1,15 @@
		public class Event extends Task {
		protected String start;

		String deadlineDesc = input.split("/")[0].split("deadline")[1].trim();
		String deadlineDay = input.split("/")[1].trim();

[ngyongjian] iP #208

Are you sure you want to change the base?

[ngyongjian] iP #208

Conversation

ngyongjian commented Feb 1, 2023

raydent30 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

YC-Michael left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

theopin left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment