[Cui Minjing] iP #13

Cuiminjing · 2021-08-27T10:00:23Z

No description provided.

giterator · 2021-09-01T03:53:00Z

src/main/java/Duke.java

 public class Duke {
-    public static void main(String[] args) {
+
+    public static final String Line = "    ____________________________________________________________";


Please refactor this attribute name as fully capitalized. All constants' names should be capitalized.

giterator · 2021-09-01T03:54:23Z

src/main/java/Task.java

@@ -0,0 +1,21 @@
+public class Task {
+    protected String content;
+    protected boolean Done;


Done could be renamed to sound like a boolean. For example, isDone

giterator · 2021-09-01T03:58:05Z

src/main/java/Duke.java

+                String[] number = command.split(" ");
+                int taskIndex = Integer.parseInt(number[1]) - 1;
+                tasks[taskIndex].complete();
+                System.out.println(Line + "\n" + "     Nice! I've marked this task as done:" + "\n     " + tasks[taskIndex].toString() + "\n" + Line);


Please make this line shorter to improve readability. The same can be done for other long lines in this method.

giterator · 2021-09-01T04:00:30Z

src/main/java/Duke.java

+        System.out.println("     What can I do for you?");
+        System.out.println(Line);
+    }
+    public static void main(String[] args) {


The implementation for this method is quite long. Perhaps it could be refactored further? For example, the handling of each task (todo / deadline / event) could be made different methods. You may also consider making the "happy path" more prominent.

giterator · 2021-09-01T04:10:02Z

src/main/java/Duke.java

+        for(int i=0; i<taskSum; i++){
+            now = tasks[i];
+            System.out.println("     " + (i+1) + "." + now.toString());


You might like to insert spaces before and after arithmetic operators too improve readability

giterator · 2021-09-01T04:16:41Z

src/main/java/Duke.java

+                        break;
+                    case "bye":
+                        break;
+                    default:


In the default case, you could either use an explicit //Fallthrough comment or include a break statement for consistent style

giterator · 2021-09-01T04:22:35Z

Apart from refactoring the main() method in Duke.java, just a few nits to fix.

Rakesh12000

First PR done.

Rakesh12000 · 2021-09-01T03:38:35Z

src/main/java/Duke.java

@@ -2,24 +2,24 @@

 public class Duke {

+    public static final String Line = "    ____________________________________________________________";
    private static Task[] tasks = new Task[110];
    private static int taskSum = 0;

    public static void printList() {
        Task now;


Could use a more appropriate variable name like task instead of now

Rakesh12000 · 2021-09-01T03:41:32Z

src/main/java/Duke.java

+                int first = command.indexOf(" ");
+                String item = command.substring(first,command.length());
+                tasks[taskSum] = new Todo(item);
+                taskSum = taskSum + 1;


Could be simplified to taskSum++;

Rakesh12000 · 2021-09-01T03:44:23Z

src/main/java/Duke.java

        }
-        System.out.println("____________________________________________________________");
+        System.out.println(Line);
    }
    public static void bye() {


Correct use of casing for method bye

Rakesh12000 · 2021-09-01T03:46:57Z

src/main/java/Task.java

@@ -14,4 +14,8 @@ public void complete(){
    public String getInformation() {


Correct use of camelCase.

xseh

Overall good job! Please consider using more SLAP to make the code better to understand and simply some of the logic (Practice KISSing) if possible.

xseh · 2021-09-17T11:13:15Z

src/main/java/Duke/Duke.java

+        Task now;
+        System.out.println(Line);
+        System.out.println("     Here are the tasks in your list:");
+        for(int i=0; i<taskSum; i++){


There should be a spacing between the brackets ( and { in the for statement. The same concept applies to the rest of the code.

xseh · 2021-09-17T11:18:16Z

src/main/java/Duke/Duke.java

+            if(command.contains("done")){
+                String[] number = command.split(" ");
+                int taskIndex = Integer.parseInt(number[1]) - 1;
+                tasks.get(taskIndex).complete();
+                System.out.println(Line + "\n" + "     Nice! I've marked this task as done:" + "\n     " +  tasks.get(taskIndex).toString() + "\n" + Line);
+            }
+            else if(command.contains("delete")){
+                String[] number = command.split(" ");
+                int taskIndex = Integer.parseInt(number[1]) - 1;
+                Task temp = tasks.get(taskIndex);
+                System.out.println(Line + "\n" + "     Noted. I've removed this task: " + "\n     " +  tasks.get(taskIndex).toString() + "\n" + "     Now you have " + taskSum + " tasks in the list");
+                System.out.println(Line);
+                tasks.remove(taskIndex);
+                taskSum--;
+            }
+            else if(command.contains("todo")){
+                int first = command.indexOf(" ");
+                int check = command.indexOf("d");
+                if(check == command.length()-2){
+                    System.out.println(Line);
+                    System.out.println("     ☹ OOPS!!! The description of a todo cannot be empty.");
+                    System.out.println(Line);
+                    continue;
+                }
+                String item = command.substring(first,command.length());
+                Task temp = new Todo(item);
+                tasks.add(temp);
+                taskSum++;
+                System.out.println(Line + "\n" + "     Got it. I've added this task: " + "\n" + "       [T][ ] " + item + "\n" + "     Now you have " + taskSum + " tasks in the list");
+                System.out.println(Line);
+
+            }
+            else if(command.contains("deadline")){
+                int first = command.indexOf(" ");
+                int check = command.indexOf("deadline");
+                if(check == command.length()-8){
+                    System.out.println(Line);
+                    System.out.println("     ☹ OOPS!!! The description of a deadline cannot be empty.");
+                    System.out.println(Line);
+                    continue;
+                }
+                int itemEnd = command.indexOf("/");
+                String item = command.substring(first,itemEnd);
+                String by = command.substring(itemEnd + 1,command.length());
+                Task temp = new Deadline(item,by);
+                tasks.add(temp);
+                taskSum++;
+                System.out.println(Line + "\n" + "     Got it. I've added this task: " + "\n" + "       [D][ ] " + item + " (" + by + ")" + "\n" + "     Now you have " + taskSum + " tasks in the list");
+                System.out.println(Line);
+            }
+            else if(command.contains("event")){
+                int first = command.indexOf(" ");
+                int check = command.indexOf("event");
+                if(check == command.length()-5){
+                    System.out.println(Line);
+                    System.out.println("     ☹ OOPS!!! The description of a event cannot be empty.");
+                    System.out.println(Line);
+                    continue;
+                }
+                int itemEnd = command.indexOf("/");
+                String item = command.substring(first,itemEnd);
+                String at = command.substring(itemEnd + 1,command.length());
+                Task temp = new Event(item,at);
+                tasks.add(temp);
+                taskSum++;
+                System.out.println(Line + "\n" + "     Got it. I've added this task: " + "\n" + "       [E][ ] " + item + " (" + at + ")" + "\n" + "     Now you have " + taskSum + " tasks in the list");
+                System.out.println(Line);
+            }
+            else{
+                switch (command){
+                    case "list":
+                        printList();
+                        break;
+                    case "bye":
+                        break;
+                    default:
+                        System.out.println(Line);
+                        System.out.println("     ☹ OOPS!!! I'm sorry, but I don't know what that means :-(");
+                        System.out.println(Line);
+                }
+            }
+        }while(!command.equals("bye"));
+        bye();
+    }
+}


The logic can be further broken down into simpler function, therefore, consider using more SLAP.

Can consider using the switch (instead of if-else) statement too to simplify the code.

Function is too bulky and long (more than 30 LOC), consider breaking it down into smaller parts.

Can consider making the happy path prominent (i.e. make the successful execution path obvious).

xseh · 2021-09-17T11:20:38Z

src/main/java/Duke/Todo.java

+
+    public Todo(String content) {
+        super(content);
+


Remove unnecessary whitespace within the code

xseh · 2021-09-17T11:24:04Z

src/main/java/Duke/Duke.java

+        greeting();
+        String command;
+        Scanner in = new Scanner(System.in);
+        String f = "data/lines.txt";


If the string is a constant, the variable name must be all uppercase (and underscore to separate the words) instead. A better variable name would also make understanding the code easier.

xseh · 2021-09-17T11:26:12Z

src/main/java/Duke/Duke.java

+            try {
+                writeToFile(f);
+            } catch (IOException e) {
+                System.out.println("Something went wrong: " + e.getMessage());


Avoid the use of magic strings. In this case, the error message can be extracted into a constant variable with a suitable name.

xseh · 2021-09-17T11:27:39Z

src/main/java/Duke/Duke.java

+            }
+            else if(command.contains("delete")){
+                String[] number = command.split(" ");
+                int taskIndex = Integer.parseInt(number[1]) - 1;


Avoid the use of magic literals. Using a suitable variable name for the constant numbers (which describes the value) would help the understanding of the program.

xseh · 2021-09-17T11:31:59Z

src/main/java/Duke/Duke.java

+                int first = command.indexOf(" ");
+                int check = command.indexOf("deadline");


Consider using a better variable name that is more descriptive. e.g. indexOfDeadline

xseh · 2021-09-17T11:38:46Z

src/main/java/Duke/Duke.java

+            }
+            else{
+                switch (command){
+                    case "list":


Case/default clauses must be on the same indentation as the switch statement.

xseh · 2021-09-17T11:40:17Z

src/main/java/Duke/Duke.java

+                        System.out.println(Line);
+                }
+            }
+        }while(!command.equals("bye"));


There should be a spacing between the bracket } and while clause

xseh · 2021-09-17T11:46:52Z

src/main/java/Duke/Duke.java

+                Task temp = new Todo(item);
+                tasks.add(temp);
+                taskSum++;
+                System.out.println(Line + "\n" + "     Got it. I've added this task: " + "\n" + "       [T][ ] " + item + "\n" + "     Now you have " + taskSum + " tasks in the list");


Line is too long. Consider separating the line into multiple lines.

test

Cuiminjing and others added 7 commits August 25, 2021 13:55

level-0

4caa62c

Update Duke.java

22361f2

Merge remote-tracking branch 'origin/master'

d9444d5

Level-1

5a95fff

Level-1

9082346

Level-2

f9d3e52

Level-3

035821c

Cuiminjing changed the title ~~improve code~~ [Cui Minjing]ip Aug 27, 2021

Cuiminjing changed the title ~~[Cui Minjing]ip~~ [Cui Minjing] iP Aug 27, 2021

Cuiminjing added 3 commits August 31, 2021 16:13

Level-4

44ec76d

Level-4

b1b1d94

Level-4

24b02c7

giterator reviewed Sep 1, 2021

View reviewed changes

Level-4 final with ui

e497d25

Rakesh12000 reviewed Sep 4, 2021

View reviewed changes

Cuiminjing added 13 commits September 5, 2021 16:35

level-4 after peer review

2222f06

Level-5

633befa

exception

e7c8ad4

package

c2637d1

commit

7caaf87

for branch level 5

5052722

for master

fe3aae2

for master

78c0a19

practice

8f91ee9

..

c6ae5c3

level-6

2a818e7

for master

548867c

level-7

dc40ec5

xseh reviewed Sep 17, 2021

View reviewed changes

Cuiminjing and others added 30 commits September 26, 2021 23:41

Update README.md

bf3ba61

Update README.md

eb1a1b4

Update README.md

912736e

Update README.md

b84167c

no message

c90176d

Merge remote-tracking branch 'origin/master'

a06dec1

images

c039eb1

Update README.md

1cde7f8

Update README.md

3798107

Update README.md

1d7f2da

Update README.md

2e7fc7d

Update README.md

ac11498

Update README.md

46bfe80

Update README.md

bf17d78

Update README.md

4c56f42

Update README.md

d0fbb00

Update README.md

05d765d

final

338ef72

add error detecting

59ebc42

Update README.md

c521047

Update README.md

b5314f1

Update README.md

5c06be4

Update README.md

7f3abf0

Update README.md

82aaf0f

test

10f2a4a

Merge pull request #1 from Cuiminjing/branch-A-JavaDoc

9080cae

test

final version of ip

12ecb0d

Merge branch 'branch-A-JavaDoc'

4b632a0

Merge remote-tracking branch 'origin/master'

93acc4c

change for ug

8a4135c

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Cui Minjing] iP #13

[Cui Minjing] iP #13

Cuiminjing commented Aug 27, 2021

giterator Sep 1, 2021

giterator Sep 1, 2021

giterator Sep 1, 2021

giterator Sep 1, 2021

giterator Sep 1, 2021

giterator Sep 1, 2021

giterator commented Sep 1, 2021

Rakesh12000 left a comment

Rakesh12000 Sep 1, 2021

Rakesh12000 Sep 1, 2021

Rakesh12000 Sep 1, 2021

Rakesh12000 Sep 1, 2021

xseh left a comment

xseh Sep 17, 2021

xseh Sep 17, 2021

xseh Sep 17, 2021

xseh Sep 17, 2021

xseh Sep 17, 2021

xseh Sep 17, 2021

xseh Sep 17, 2021

xseh Sep 17, 2021

xseh Sep 17, 2021

xseh Sep 17, 2021

		@@ -14,4 +14,8 @@ public void complete(){
		public String getInformation() {

		int first = command.indexOf(" ");
		int check = command.indexOf("deadline");

[Cui Minjing] iP #13

Are you sure you want to change the base?

[Cui Minjing] iP #13

Conversation

Cuiminjing commented Aug 27, 2021

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

giterator commented Sep 1, 2021

Rakesh12000 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

xseh 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

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