[wingho]IP by kum-wh · Pull Request #196 · nus-cs2113-AY2122S1/ip

kum-wh · 2021-09-02T03:05:37Z

No description provided.

Change the initial string that printed

…t on bye Import a Scanner class and add a to that echo input.

…hen list is called Add a array of size 100 that store string

Add Task class iinstead of just storing the tasks as strings

Added 3 class that inherit from

joshualeeky

Just some minor comments :)

joshualeeky · 2021-09-02T04:34:45Z

src/main/java/Duke.java

+                for (int i = 0; i < listIndex; i++) {
+                    System.out.println((i + 1) + ". " + list[i].toString());
+                }
+            }else if(echoLower.startsWith("event")){


Just a small issue, I think there should be spaces before "{" and after "}"

joshualeeky · 2021-09-02T04:37:18Z

src/main/java/Duke.java

+                Task t = new Deadline(description, deadline);
+                list[listIndex] = t;
+                listIndex += 1;
+                System.out.println("Got it. I've added this task: " + System.lineSeparator() + t.toString() + System.lineSeparator() + "Now you have " + listIndex +" tasks in the list.");


I think there are more than 120 characters in this line, maybe you could split it into 2 lines?

joshualeeky · 2021-09-02T04:38:03Z

src/main/java/Duke.java

+                Task t = new Event(description, time);
+                list[listIndex] = t;
+                listIndex += 1;
+                System.out.println("Got it. I've added this task: " + System.lineSeparator() + t.toString() + System.lineSeparator() + "Now you have " + listIndex+" tasks in the list.");


I think there are more than 120 characters in this line, maybe you could split it into 2 lines?

joshualeeky · 2021-09-02T04:38:09Z

src/main/java/Duke.java

+                Task t = new Todo(description);
+                list[listIndex] = t;
+                listIndex += 1;
+                System.out.println("Got it. I've added this task: " + System.lineSeparator() + t.toString() + System.lineSeparator() + "Now you have " + listIndex +" tasks in the list.");


I think there are more than 120 characters in this line, maybe you could split it into 2 lines?

joshualeeky · 2021-09-02T04:39:17Z

src/main/java/Duke.java

+                " What can I do for you?\n" +
+                "____________________________________________________________\n");
+        Scanner in = new Scanner(System.in);
+        Task[] list = new Task[100];


Maybe can try using another name for the list? Just to make it clear what is being stored in here.

joshualeeky · 2021-09-02T04:42:07Z

src/main/java/Task.java

+        return description;
+    }
+    public String getStatusIcon(){
+        return(isDone?"X":" ");


Maybe you could consider using the full if statement.

Yuxinn-J

LGTM, just a few issues about coding standard need change

Yuxinn-J · 2021-09-02T05:44:16Z

src/main/java/Task.java

+        return description;
+    }
+    public String getStatusIcon(){
+        return(isDone?"X":" ");


Suggested change

return(isDone?"X":" ");

return(isDone ? "X" : " ");

Yuxinn-J · 2021-09-02T05:47:42Z

src/main/java/Duke.java

+                }
+            }else if(echoLower.startsWith("event")){
+                int startOfTime = echoLower.indexOf("/");
+                String description = echoLower.substring(6,startOfTime);


it is better to avoid magic number, e.g., 6

it is better to avoid magic numbers, e.g., 6

Yuxinn-J · 2021-09-02T05:52:24Z

src/main/java/Duke.java

+                for(int i = 0; i < listIndex; i ++){
+                    System.out.println((i + 1) + ". " + list[i].toString());
+                }
+            }else if(echoLower.equals("done")) {


Just a very small issue of coding standard. Based on Checkstyle, there should be a space before else and after if here.

Suggested change

}else if(echoLower.equals("done")) {

} else if (echoLower.equals("done")) {

ruyian

Overall great job, Wingho!

ruyian · 2021-09-02T08:42:16Z

src/main/java/Deadline.java

+public class Deadline extends Task{
+    protected String by;
+    public Deadline(String description, String by){
+        super(description);
+        this.by = by;
+    }
+    public String toString(){
+        return "[D]" + super.toString() + "by: " + by + ")";
+    }


It is better to follow the coding standard where there is a space between the class name and the {. The same goes for methods.

ruyian · 2021-09-02T08:43:02Z

src/main/java/Event.java

@@ -0,0 +1,10 @@
+public class Event extends Task{
+    protected String time;


It is better to make it a private string to ensure data hiding in encapsulation.

ruyian · 2021-09-02T08:44:06Z

src/main/java/Task.java

@@ -15,4 +15,7 @@ public String getDescription(){
    public String getStatusIcon(){
        return(isDone?"X":" ");


Please follow the coding standard in leaving spaces.

ruyian · 2021-09-02T08:45:15Z

src/main/java/Deadline.java

+    public String toString(){
+        return "[D]" + super.toString() + "by: " + by + ")";


It might be more readable if you add the @OverRide before overriding toString() method.

ruyian · 2021-09-02T08:45:46Z

src/main/java/Duke.java

            }else if(echoLower.equals("list")) {
                for(int i = 0; i < listIndex; i ++){
-                    System.out.println((i + 1) + ". ["+ list[i].getStatusIcon() + "] " + list[i].getDescription());
+                    System.out.println((i + 1) + ". " + list[i].toString());
                }
            }else if(echoLower.equals("done")) {


Please leave spaces for the if-else blocks, abiding by the coding standard.

ruyian · 2021-09-02T08:46:35Z

src/main/java/Deadline.java

+    public Deadline(String description, String by){
+        super(description);
+        this.by = by;
+    }
+    public String toString(){


I think we need to have a blank line in between variables declared and method, as well as between two methods.

Improve the naming to be more descriptive of the variable it represent Use constants to remove the need for magic numbers Change array into arraylist to prevent overflow of task in the array

…y to set multiple task as done in the same line Added a interface to listManager to make it easier to know what listMananger does

…n user input Added a class artbot that use 2D array to create and merge the string to form the shape of letters

…n delete is called

Added a file class to interact with txt files

# Conflicts: # src/main/java/duke/ListManager.java

rafaelperes

Hi Wing Ho! Nice job here. I hope my comments might help you :)

rafaelperes · 2021-09-19T06:30:30Z

src/main/java/duke/Duke.java

+        fileManager.covertStringToTask();
+        while(isOnline) {
+            String userInput = in.nextLine().toLowerCase().trim();
+                if (userInput.startsWith("!")) {


perhaps a magic string here? maybe change to a named constant in case you are going to use it somewhere else...

rafaelperes · 2021-09-19T06:33:11Z

src/main/java/duke/FileManager.java

+            String[] splitString = s.split(";");
+            switch (splitString[0]){
+            case "[T]":
+                listManager.addTodo(splitString[2], true);
+                break;
+            case "[E]":
+                listManager.addEvent(splitString[2],splitString[3], true);
+                break;
+            case"[D]":
+                listManager.addDeadline(splitString[2],splitString[3], true);


what does splitString[0], splitString[2], splitString[3] stands for? perhaps declare it giving specific names to improve understandability? e.g. splitString[0] -> taskType?

rafaelperes · 2021-09-19T06:33:42Z

src/main/java/duke/FileManager.java

+            case"[D]":
+                listManager.addDeadline(splitString[2],splitString[3], true);
+            }
+            if (splitString[1].equals("[X]")){


perhaps change [X] to a constant?

rafaelperes · 2021-09-19T06:37:50Z

src/main/java/duke/ListManager.java

+            System.out.println(Logo.dividerWithoutNewLine);
+            printAddItem(t);
+            System.out.println(Logo.dividerWithoutNewLine);
+        }


perhaps improve understandability by apply the SLAP concept here? e.g a method to print these 3 lines of code?

rafaelperes · 2021-09-19T06:38:19Z

src/main/java/duke/ListManager.java

+            System.out.println(Logo.dividerWithoutNewLine);
+            printAddItem(t);
+            System.out.println(Logo.dividerWithoutNewLine);


same as line 99, repeated code here...

A-JavaDoc

Level:8

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

wingho2 added 5 commits September 2, 2021 02:57

Change the dukes speech

cfa100b

Change the initial string that printed

Add a echo functions that repeat commands entered by the user and exi…

58398a4

…t on bye Import a Scanner class and add a to that echo input.

Add the ability to store text input into a list and display it back w…

7b1d074

…hen list is called Add a array of size 100 that store string

Add the ability to mark task as done

1d74b39

Add Task class iinstead of just storing the tasks as strings

Add catergory to the task, todo,deadlines and events

eed209d

Added 3 class that inherit from

joshualeeky reviewed Sep 2, 2021

View reviewed changes

Yuxinn-J reviewed Sep 2, 2021

View reviewed changes

ruyian reviewed Sep 2, 2021

View reviewed changes

wingho2 added 18 commits September 4, 2021 11:59

Refactor the code to remove duplcated functions

f4fecb4

Improve the naming to be more descriptive of the variable it represent Use constants to remove the need for magic numbers Change array into arraylist to prevent overflow of task in the array

Added the ability to add mutiple task in the same line and the abilit…

b8ad62d

…y to set multiple task as done in the same line Added a interface to listManager to make it easier to know what listMananger does

Added a way to print words like what was printed for duke but based o…

37abf0d

…n user input Added a class artbot that use 2D array to create and merge the string to form the shape of letters

Added testing

1e6963b

Add a lot of exceptions to prevent any errors

2f491b7

Changing some Strings

4a08901

Further fixes to the String printed

50ed25d

Merge branch 'branch-Level-5'

64b7752

Minimal packaging

2305316

More packages

57771c8

Fix packages

a9d7713

Merge branch 'branch-A-Packages'

1a2a6ac

Add a delete function to class ListManager and class inputhandler whe…

c383114

…n delete is called

Some formatting words changes

5ee47d4

Add the save function to txt files

6ee2be3

Added a file class to interact with txt files

Merge branch 'branch-Level-7'

f714fce

# Conflicts: # src/main/java/duke/ListManager.java

remove some bugged files

ec11a9f

debugging the file update when there is no task in the array

6d21d77

rafaelperes reviewed Sep 19, 2021

View reviewed changes

wingho2 added 3 commits September 25, 2021 16:57

Change all the commands into their own class

8565495

Added a find date command that find task with a specific date

80e1c4e

Added Find Keyword to search the list

71fb4c5

kum-wh and others added 30 commits September 28, 2021 16:58

Merge branch 'master' into branch-A-JavaDoc

025d7c0

Merge pull request #9 from kum-wh/branch-A-JavaDoc

c922374

A-JavaDoc

Level:8

ecceb76

Level:9

8e59389

A-JavaDoc

887ddf1

Merge pull request #10 from kum-wh/branch-Level-8

b184fca

Level:8

Merge branch 'branch-A-JavaDoc'

2989450

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

Solving merge conflict

d07b2aa

Merge branch 'branch-Level-9'

bd7aca4

Merging PRS

f3317e1

Fix Exception Messages and add more comments to functions

264344e

Fix a bug that invalid date keep throwing parsing error

21ea138

Update README.md

19b892c

Set theme jekyll-theme-cayman

8e97fa8

Update README.md

41d7353

Create README.md

5d3bffc

Update README.md

4339745

Update README.md

7576a85

Update README.md

3f424fc

Update README.md

5cbdada

Update README.md

0ec1b61

Update README.md

28c7be4

Update README.md

e4d14a7

Update README.md

4054b00

Update README.md

bc4554c

Update README.md

1816abe

Update README.md

e29bcf7

Update README.md

cee51f8

Fix ArtBot null pointer

bba48ca

Merge branch 'master' of https://github.com/kum-wh/ip

f2c2a13

	}else if(echoLower.equals("done")) {
	} else if (echoLower.equals("done")) {

		@@ -0,0 +1,10 @@
		public class Event extends Task{
		protected String time;

		@@ -15,4 +15,7 @@ public String getDescription(){
		public String getStatusIcon(){
		return(isDone?"X":" ");

		public String toString(){
		return "[D]" + super.toString() + "by: " + by + ")";

Conversation

kum-wh commented Sep 2, 2021

Uh oh!

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

Yuxinn-J 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!

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

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

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants