[TJ-Hoo] iP by TJ-Hoo · Pull Request #190 · nus-cs2113-AY2223S2/ip

TJ-Hoo · 2023-01-31T09:24:29Z

No description provided.

This reverts commit a1fb5b3.

This reverts commit 8f06dc1.

Stella1585

Looks good, you followed the coding standards quite well. Overall, you can try to find more suitable variable names that helps to get your intention across. Also the nesting is quite deep for the mark/unmark section. Lastly you can try extracting methods from main as it is quite long.

Stella1585 · 2023-02-01T07:40:11Z

+                    System.out.print(i+1+".");
+                    System.out.println(taskList.get(i).markTask()+taskList.get(i).name);
+                }
+            } else if(userInput.contains("unmark") || userInput.contains("mark")){


(not coding standard related but) Since you are using contains and not equals, do you think contains(unmark) is necessary?

Stella1585 · 2023-02-01T07:44:57Z

+                }
+            } else if(userInput.contains("unmark") || userInput.contains("mark")){
+                Integer itemNumber = new Integer(0);
+                String [] IndexArr = userInput.split(" ",2);


this is a variable so camel case should be used, also maybe a better variable name can be used as this array contains both the command and the index.

Stella1585 · 2023-02-01T07:47:52Z

-        boolean isFinished = false;
-        List toDoList = new List();
+        Scanner in = new Scanner(System.in);
+        ArrayList<Task> taskList = new ArrayList<Task>();


plural form should be used for collection of objects, so perhaps using names such as tasks would be better

Stella1585 · 2023-02-01T08:06:55Z

+            if(userInput.equals("list")){
+                for(int i = 0; i<taskList.size(); i++){
+                    System.out.print(i+1+".");
+                    System.out.println(taskList.get(i).markTask()+taskList.get(i).name);


for your function .get, perhaps be clearer about what you are getting, such using as getStatus or getName.

Stella1585 · 2023-02-01T08:10:35Z

+            if(userInput.equals("bye")){
+                break;
+            }
+            if(userInput.equals("list")){


any reason why you did not continue with else if?

Manoj364 · 2023-02-01T12:42:34Z

+            if(userInput.equals("bye")){
+                break;
+            }
+            if(userInput.equals("list")) {
+                for (int i = 0; i < taskList.size(); i++) {
+                    System.out.print(i + 1 + ".");
+                    System.out.println(taskList.get(i).toString());
+                }
+            } else if (userInput.contains("todo")) {
+                String info = userInput.substring(5).trim();
+                taskList.add(new Todo(info));
+                int finalTask = taskList.size() - 1;
+                System.out.println("Got it. I've added this task:");
+                System.out.println(taskList.get(finalTask).toString());
+                if (taskList.size() == 1){
+                    System.out.println("Now you have " +taskList.size()+" task in the list.");
+                } else {
+                    System.out.println("Now you have " + taskList.size() + " tasks in the list.");
+                }
+            } else if (userInput.contains("deadline")) {
+                String [] listArray = userInput.split("/",2);
+                String description = listArray[0];
+                String dueDate = listArray[1];
+                String info = description.substring(8).trim();
+                String due = dueDate.substring(3).trim();
+                taskList.add(new Deadline(info, due));
+                int finalTask = taskList.size() - 1;
+                System.out.println("Got it. I've added this task:");
+                System.out.println(taskList.get(finalTask).toString());
+                if (taskList.size() == 1){
+                    System.out.println("Now you have " +taskList.size()+" task in the list.");
+                } else {
+                    System.out.println("Now you have " + taskList.size() + " tasks in the list.");
+                }
+            } else if (userInput.contains("event")) {
+                String [] listArray = userInput.split("/",3);
+                String description = listArray[0];
+                String startTime = listArray[1];
+                String endTime = listArray[2];
+                String info = description.substring(6).trim();
+                String start = startTime.substring(5).trim();
+                String end = endTime.substring(3).trim();
+                taskList.add(new Event(info, start, end));
+                int finalTask = taskList.size() - 1;
+                System.out.println("Got it. I've added this task:");
+                System.out.println(taskList.get(finalTask).toString());
+                if (taskList.size() == 1){
+                    System.out.println("Now you have " +taskList.size()+" task in the list.");
+                } else {
+                    System.out.println("Now you have " + taskList.size() + " tasks in the list.");
+                }
+            } else if(userInput.contains("unmark") || userInput.contains("mark")){
+                Integer itemNumber;
+                String [] IndexArr = userInput.split(" ",2);
+                itemNumber = Integer.parseInt(IndexArr[1])-1;
+                if (userInput.contains("unmark")) {
+                    System.out.println("OK, I've marked this task as not done yet:");
+                    taskList.get((int) itemNumber).isCompleted = false;
+                    System.out.println(taskList.get(itemNumber).toString());
+                }
+                else {
+                    System.out.println("Nice! I've marked this task as done:");
+                    taskList.get(itemNumber).isCompleted = true;
+                    System.out.println(taskList.get(itemNumber).toString());
+                }
+            } else {
+                taskList.add(new Task(userInput,false));
+                System.out.println("added: "+userInput);
+            }


I like how you avoided deep-nesting

Manoj364 · 2023-02-01T12:47:32Z

+                int finalTask = taskList.size() - 1;
+                System.out.println("Got it. I've added this task:");
+                System.out.println(taskList.get(finalTask).toString());
+                if (taskList.size() == 1){


Perhaps you can leave a space between your round bracket and curly bracket, so that your formatting looks consistent

Manoj364 · 2023-02-01T12:54:32Z

+
+        String userInput;
+        while(in.hasNext()){
+


Perhaps this space can be removed

Manoj364 · 2023-02-01T13:00:29Z

+                if (taskList.size() == 1){
+                    System.out.println("Now you have " +taskList.size()+" task in the list.");
+                } else {
+                    System.out.println("Now you have " + taskList.size() + " tasks in the list.");
+                }


You can consider refactoring this portion of code into a method as it is repeated multiple times in the if-else statements

Manoj364 · 2023-02-01T13:04:02Z

+                System.out.println("Got it. I've added this task:");
+                System.out.println(taskList.get(finalTask).toString());
+                if (taskList.size() == 1){
+                    System.out.println("Now you have " +taskList.size()+" task in the list.");


Perhaps you can include spaces like this to be more consistent in your formatting

Suggested change

System.out.println("Now you have " +taskList.size()+" task in the list.");

System.out.println("Now you have " + taskList.size() + " task in the list.");

slightlyharp

Good job in term of code standard. However there are still room for improvement for readability. Please take note of the naming of variables, improve on abstractions and remember to give comments to your code.

slightlyharp · 2023-02-12T16:38:37Z

@@ -0,0 +1,11 @@
+public class Deadline extends Task {
+	protected String due;


Use noun for variable. Perhaps dueDate is a better name for it.

slightlyharp · 2023-02-12T16:46:31Z

+                String [] listArray = userInput.split("/",2);
+                String description = listArray[0];
+                String dueDate = listArray[1];
+                String info = description.substring(8).trim();


Consider avoid all these magic numbers, use named constant instead.
Perhaps use space to split up the phrase instead of hardcoding the character position.

slightlyharp · 2023-02-12T16:49:26Z

@@ -0,0 +1,11 @@
+public class Deadline extends Task {
+	protected String due;
+	public Deadline(String info, String due) {


Perhaps name is a better naming for the variable instead of info

slightlyharp · 2023-02-12T17:09:21Z

+                    System.out.println("Now you have " + tasks.size() + " tasks in the list.");
+                }
+            } else if (userInput.contains("deadline")) {
+                String [] listArray = userInput.split("/",2);


Consider changing to a more meaningful name such as phrases

slightlyharp · 2023-02-12T17:14:32Z

+        String userInput;
+        while(in.hasNext()){
+            userInput = in.nextLine();
+            if(userInput.equals("bye")){


Leave a space before the open curly bracket

slightlyharp · 2023-02-12T17:18:16Z

+                if (tasks.size() == 1) {
+                    System.out.println("Now you have " + tasks.size() + " task in the list.");
+                } else {
+                    System.out.println("Now you have " + tasks.size() + " tasks in the list.");


Consider creating a method for this since it is used a few times in the code

slightlyharp · 2023-02-12T17:22:52Z

+                    System.out.println("Now you have " + tasks.size() + " tasks in the list.");
+                }
+            } else if(userInput.contains("mark")){
+                Integer itemNumber;


You can still use normal int data type

slightlyharp · 2023-02-12T17:28:46Z

+                String [] listArray = userInput.split("/",3);
+                String description = listArray[0];
+                String startTime = listArray[1];
+                String endTime = listArray[2];


User might not give the desired input. Consider implement some error handling taught in week 5

slightlyharp · 2023-02-12T17:37:23Z

+	public Todo (String info) {
+		super (info);
+	}
+	public String toString() {


Perhaps adding override annotation to indicate that the method is overriding a method from the parent class. Be consistent in the annotations for all the classes.

slightlyharp · 2023-02-12T17:44:01Z

+        }
    }
-}
+}


Relatively long and deep nested code. Consider break up the code with more abstractions. (create method for segments that are used repeatedly and a method for each command)

# Conflicts: # src/main/java/Duke.java

Level-9

A-JavaDoc

# Conflicts: # src/main/java/Duke.java

TJ-Hoo added 8 commits January 25, 2023 13:40

no message

a1fb5b3

Revert "no message"

50e9438

This reverts commit a1fb5b3.

no message

8f06dc1

Revert "no message"

15e6f91

This reverts commit 8f06dc1.

Level 0

f19569e

Level 1

aeb3c02

Level 2

8b22744

Level 3

1feb6e5

Stella1585 reviewed Feb 1, 2023

View reviewed changes

Level 4

7c31d51

Manoj364 reviewed Feb 1, 2023

View reviewed changes

TJ-Hoo added 2 commits February 9, 2023 21:43

A-CodingStandard

1bf5e36

A-CodingQuality

3d6f6e3

slightlyharp reviewed Feb 12, 2023

View reviewed changes

TJ-Hoo and others added 16 commits February 16, 2023 15:21

Level-5

875ec03

Level-6

2f9f975

Level-7

6bdd653

Merge branch 'branch-level-7'

e1391de

# Conflicts: # src/main/java/Duke.java

Level-9

8206b33

Merge pull request #1 from TJ-Hoo/branch-level-9

2e3fd7c

Level-9

A-JavaDoc

4375e13

Merge pull request #2 from TJ-Hoo/branch-JavaDoc

905f7d0

A-JavaDoc

Update README.md

1ebdea6

Update README.md

31322c7

no message

3986655

Rename README.md to UserGuide

9c0ba57

Merge remote-tracking branch 'origin/master'

f221b77

# Conflicts: # src/main/java/Duke.java

Merge remote-tracking branch 'origin/master'

ce6d09e

Update README.md

c25b455

Update README.md

f8faf8e

TJ-Hoo and others added 6 commits February 26, 2023 11:47

Update README.md

9366fd6

Minor Fixes

4eb7c4a

Merge remote-tracking branch 'origin/master'

24afabf

Minor improvements

3f092ce

Minor bug fixes

dc79a03

Minor fix

fd2c8f9

	System.out.println("Now you have " +taskList.size()+" task in the list.");
	System.out.println("Now you have " + taskList.size() + " task in the list.");

		@@ -0,0 +1,11 @@
		public class Deadline extends Task {
		protected String due;

+                      }
                   }
-              }
+              }

                
                    No newline at end of file

Conversation

TJ-Hoo commented Jan 31, 2023

Uh oh!

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

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

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants