[Yingchao Dai] iP #102

baggiiiie · 2021-02-04T17:53:21Z

No description provided.

This reverts commit 9b34591.

This reverts commit def7baf.

AlexanderTanJunAn

Overall goodjob! Just some small code standard violations!

AlexanderTanJunAn · 2021-02-05T01:20:18Z

src/main/java/Duke.java

+                continue;
+            } else if (inputStringSplit[0].equalsIgnoreCase("done")){
+                try{
+                    int tempindex = Integer.parseInt(inputStringSplit[1]);


Although its a small variable, names should be in camelCase

AlexanderTanJunAn · 2021-02-05T01:20:49Z

src/main/java/Duke.java

 public class Duke {
+    public static String inputString;
+    public static int listindex = 0;


camelCase here too!

AlexanderTanJunAn · 2021-02-05T01:22:11Z

src/main/java/Duke.java

+        }while(!inputString.equalsIgnoreCase("bye"));
+    }
+
+    public static void AddIntoList(){


Methods should be written in camelCase as well

AlexanderTanJunAn · 2021-02-05T01:22:46Z

src/main/java/Duke.java

 public class Duke {
+    public static String inputString;
+    public static int listindex = 0;
+    public static String list[] = new String[100];


Collection of tasks should be in plural form i.e lists

lowwilliam

Overall good job, some small detail can be improved!

lowwilliam · 2021-02-05T02:40:14Z

src/main/java/Duke.java

+        greetings();
+        StartingMenu();
+        goodbye();
+    }


Can leave one line space between two boxes of codes!

lowwilliam · 2021-02-05T02:41:31Z

src/main/java/Duke.java

+    public static int listindex = 0;
+    public static String list[][] = new String[100][5];
+    //public static String isDone[] = new String[100];
+    //public static String tasktype[] = new String[100];


Can delete the useless comments code

lowwilliam · 2021-02-05T02:43:31Z

src/main/java/Duke.java

+    public static void PrintList(int startIndex, int endIndex) {
+        for (int i = startIndex; i < endIndex; i++) {
+            System.out.println(i + 1 + ": [" + list[i][0] + "][" + list[i][1] + "]:" + " " + list[i][2] +list[i][3]);
+        }


can change i to some words meaningful

lowwilliam · 2021-02-05T02:44:29Z

src/main/java/Duke.java

+        do {
+            Scanner userinput = new Scanner(System.in);
+            inputString = userinput.useDelimiter("\\A").nextLine();
+


Can delete this empty line

BlubberMonster

Generally, your code had clear and easy to read. I liked that you used meaningful words to name your variables and methods. I noted a few issues pertaining to code quality and coding standards. You can also consider splitting into smaller classes if possible.

BlubberMonster · 2021-02-05T12:10:19Z

src/main/java/Duke.java

+            }
+            System.out.println(inputString);
+            System.out.println("____________________________________________________________");
+        }while(!inputString.equalsIgnoreCase("bye"));


Perhaps you can insert a space between the '}' and "while"?

BlubberMonster · 2021-02-05T12:14:43Z

src/main/java/Duke.java

+
+
+
+    public static void greetings(){


I like that you modularize your program. It definitely increases the readability.

BlubberMonster · 2021-02-05T12:18:33Z

src/main/java/Duke.java

+        }
+    }
+
+    public static void MarkAsDone(String donetask){


Perhaps you might consider putting this method in another class ?

BlubberMonster · 2021-02-05T12:29:15Z

src/main/java/Duke.java

+            PrintList(0, listindex);
+        }
+    }
+


You can consider deleting the lines here.

BlubberMonster · 2021-02-05T12:31:05Z

src/main/java/Duke.java

+    //    listindex += 1;
+    //}
+//
+    public static void PrintList(int startIndex, int endIndex) {


I like how you used meaningful words to name your parameters.

This reverts commit a2f6d98.

This reverts commit 7a75c08.

rabhijit

Overall, good work! In terms of code readability, I think you could improve on simplifying/breaking down overly long methods, reducing long lines of code and reducing deep if-else nesting. But otherwise, your code is organised and readable. Consider including comments to explain what your code is doing as well.

rabhijit · 2021-02-21T04:13:17Z

src/main/java/Duke/Duke.java

+import java.util.List;
+import java.util.Scanner;
+import java.util.ArrayList;
+import java.io.*;


It's best to avoid wildcard imports, as seen here

rabhijit · 2021-02-21T04:14:41Z

src/main/java/Duke/Duke.java

+import java.io.*;
+
+public class Duke {
+    private static String LINE = "____________________________________________________________";


Reserve uppercase variable names for constants (i.e. static final variables)

rabhijit · 2021-02-21T04:16:39Z

src/main/java/Duke/Duke.java

+        }
+    }
+
+    private static void readFile(List<Task> lists){


Perhaps you could restructure your functions to avoid deep if-else nesting, to increase its readability and make debugging easier for yourself!

rabhijit · 2021-02-21T04:23:19Z

src/main/java/Duke/Duke.java

+                if (taskInList instanceof ToDoTask) {
+                    writer.write(taskInList.getTaskType() + "|" + taskInList.isDone() + "|" + taskInList.getTask().trim());
+                } else {
+                    writer.write(taskInList.getTaskType() + "|" + taskInList.isDone() + "|" + taskInList.getTask().trim() + "|" + taskInList.getTaskTime().trim());


If your code lines are getting overly long, it's best to break it down into multiple lines (e.g. you could save your variables before writing them)

rabhijit · 2021-02-21T04:25:09Z

src/main/java/Duke/Duke.java

+                        if (isDone.equals("true")) {
+                            taskInFile.markAsDone();
+                        }
+                    } else {


Try and explicitly state all what all your if and else-if conditions, and leave the 'else' statement for error handling mainly

* 'master' of https://github.com/baggiiiie/ip: Update README.md Update README.md Update README.md Update README.md Update README.md Update README.md Update README.md Update README.md Update README.md Update README.md Update README.md Update README.md Update README.md Update README.md Update README.md Update README.md Update README.md Update README.md Update README.md # Conflicts: # README.md

baggiiiie added 7 commits February 4, 2021 21:56

test output

def7baf

push lrq

9b34591

Revert "push lrq"

1a255f8

This reverts commit 9b34591.

Revert "test output"

3bd42ba

This reverts commit def7baf.

Update Echo

a55de07

Update "Add, List"

27b1402

Update "Mark As Done"

b16501a

AlexanderTanJunAn reviewed Feb 5, 2021

View reviewed changes

Update "Deadline, ToDo, Event"

7007b9d

lowwilliam reviewed Feb 5, 2021

View reviewed changes

BlubberMonster reviewed Feb 5, 2021

View reviewed changes

baggiiiie added 16 commits February 6, 2021 01:50

Modify "Deadline, ToDo, Event"

0253ccb

Update Level 5

b525596

Modify Level 5

a2f6d98

Revert "Modify Level 5"

7a75c08

This reverts commit a2f6d98.

Revert "Revert "Modify Level 5""

0d39748

This reverts commit 7a75c08.

Modify Level 5

f5a00f8

Modify Level-5

d0bdb84

Update A-Exceptions

63cb05d

Update "A-Exceptions"

97f2bc9

Update A-Packages

14f1c35

Update A-Packages

4457de7

Update A-CodingStandard

1712cbc

Update Level 6

8ed9fd0

Update A-Collections

9e36969

Update Level-7

f9bdd21

Update A-Jar

84f3aee

rabhijit reviewed Feb 21, 2021

View reviewed changes

baggiiiie added 2 commits February 21, 2021 19:23

Update Level-8

ec10e0f

Modify Code Quality According to Comments

d248b2f

baggiiiie and others added 30 commits March 5, 2021 23:37

Set theme jekyll-theme-tactile

e8a3f6c

Update README.md

a625627

Set theme jekyll-theme-cayman

1cdf453

Update README.md

03e0e81

Update README.md

7ec9556

Update README.md

9fa6204

Update README.md

091da1b

Update README.md

69a467a

Update README.md

270eff0

Set theme jekyll-theme-minimal

0addc12

Set theme jekyll-theme-slate

517f4cb

Update README.md

37014de

Update README.md

916bb03

Merge branch 'master' of https://github.com/baggiiiie/ip

61f6a7e

Update README.md

f1d674e

Update README.md

853af68

Update README.md

14940c0

Update README.md

474da62

Update README.md

c7ed7b8

Update README.md

f8581eb

Update README.md

c5fd66b

Update README.md

edbb18e

Update README.md

9a6080a

Update README.md

09e680e

Update Code

c4b812e

Update README.md

ba3f6b7

Update README.md

207ba58

Update README.md

ea8aa3f

Update README.md

11b75a4

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Yingchao Dai] iP #102

[Yingchao Dai] iP #102

baggiiiie commented Feb 4, 2021

AlexanderTanJunAn left a comment

AlexanderTanJunAn Feb 5, 2021

AlexanderTanJunAn Feb 5, 2021

AlexanderTanJunAn Feb 5, 2021

AlexanderTanJunAn Feb 5, 2021

lowwilliam left a comment

lowwilliam Feb 5, 2021

lowwilliam Feb 5, 2021

lowwilliam Feb 5, 2021

lowwilliam Feb 5, 2021

BlubberMonster left a comment

BlubberMonster Feb 5, 2021

BlubberMonster Feb 5, 2021

BlubberMonster Feb 5, 2021

BlubberMonster Feb 5, 2021

BlubberMonster Feb 5, 2021

rabhijit left a comment •

edited

Loading

rabhijit Feb 21, 2021

rabhijit Feb 21, 2021

rabhijit Feb 21, 2021

rabhijit Feb 21, 2021

rabhijit Feb 21, 2021

[Yingchao Dai] iP #102

Are you sure you want to change the base?

[Yingchao Dai] iP #102

Conversation

baggiiiie commented Feb 4, 2021

AlexanderTanJunAn 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

lowwilliam 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

BlubberMonster 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

rabhijit left a comment • edited Loading

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

rabhijit left a comment •

edited

Loading