[Kuek Yan Ling] iP by yanlingkuek · Pull Request #248 · nus-cs2103-AY2021S2/ip

yanlingkuek · 2021-01-31T23:21:20Z

No description provided.

vvan-essa · 2021-02-01T00:16:55Z

+                System.out.println("added: " + input);
+            } else {
+                for (int i = 1; i <= lst.size(); i++) {
+                    System.out.println(i + ". " + lst.get(i-1));


Perhaps there should be whitespace on the left and right of the "-" sign? (:
list.get(i - 1);

vvan-essa · 2021-02-01T00:17:43Z

@@ -1,10 +1,189 @@
+import java.util.*;


Perhaps imported classes should be listed explicitly? (:

vvan-essa · 2021-02-01T00:18:46Z

+    }
+
+
+    static void level2() {


Perhaps the method names could be more descriptive? In verb form? (:

vvan-essa · 2021-02-01T00:24:16Z

+
+    }
+
+


Perhaps more javadoc comments for the methods? (:

markuz5116

I personally think that access modifier should be included for all methods so that readers can know whether the method should be accessed from outside the class, can be clearer for the readers.

markuz5116 · 2021-02-02T09:11:24Z

+
+    protected String deadline;
+
+    public DeadlineTask(String description, String deadlline) {


Perhaps there is a typo for a variable name for deadline rather than deadlline.

markuz5116 · 2021-02-02T09:15:11Z

@@ -1,10 +1,189 @@
+import java.util.*;


importing tasks individually can be clearer.

markuz5116 · 2021-02-02T09:16:17Z

+
 public class Duke {
+
+    static void level1() {


Perhaps having a method name that briefly describes what it does would be clearer.

markuz5116 · 2021-02-02T09:17:54Z

+                System.out.println("added: " + input);
+            } else {
+                for (int i = 1; i <= lst.size(); i++) {
+                    System.out.println(i + ". " + lst.get(i-1));


Not sure if I liked that the - does not have whitespace before and after it.

markuz5116 · 2021-02-02T09:19:05Z

+    }
+
+
+    static void level3() {


I would personally an access modifier for my method to show whether it can be used outside the package or class.

markuz5116 · 2021-02-02T09:19:31Z

+
+
+    static void level3() {
+        List<Task> lst = new ArrayList<>();


Perhaps instead of lst, using tasks could be clearer to know what the list contains.

markuz5116 · 2021-02-02T09:20:10Z

+
+        Scanner sc = new Scanner(System.in);
+        String input = sc.nextLine();
+        String[] inputArray = input.split(" ");


perhaps rather than inputArray, you can have a plural form of the input to show that it has multiple parts.

markuz5116 · 2021-02-02T09:22:00Z

+                }
+            } else if (inputArray[0].equals("todo") || inputArray[0].equals("deadline") || inputArray[0].equals("event")) { //adding a task
+                Task task = new Task("");
+                String[] inputArr1;


perhaps extracting all the code that deals with different types of tasks could make your code clearer.

markuz5116 · 2021-02-02T09:23:27Z

@@ -0,0 +1,21 @@
+public class Task {
+    protected String description;
+    protected boolean isDone;


I like the name of this boolean, clearly tells me what it does.

NiniJiaying · 2021-02-02T15:47:02Z

+
+    protected String deadline;
+
+    public DeadlineTask(String description, String deadlline) {


Perhaps there can be Javadoc comments to tell readers how this method works.

NiniJiaying · 2021-02-02T15:48:36Z

+
 public class Duke {
+
+    static void level1() {


Perhaps it would be clearer if this method name starts with a verb?

NiniJiaying · 2021-02-02T15:51:50Z

+        Scanner sc = new Scanner(System.in);
+        String input = sc.nextLine();
+
+        List<String> lst = new ArrayList<>();


Perhaps the parameter name can be something like commands? lst may sound a bit unclear to readers.

NiniJiaying · 2021-02-02T15:53:38Z

@@ -0,0 +1,15 @@
+public class DeadlineTask extends Task {


Maybe you can consider putting classes into packages.

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

…ExitCommand, DoneCommand, TaskdateCommand (Complete A-MoreOOP)

# Conflicts: # src/main/java/duke/Storage.java # src/main/java/duke/task/TaskList.java

…e current task list. (Complete BCD-Extension, C-DetectDuplicates)

Add assertions (Complete A-Assertions)

…h-BCD-Extension # Conflicts: # data/duke.txt

# Conflicts: # data/duke.txt # src/main/java/duke/Duke.java # src/main/java/duke/task/TaskList.java

# Conflicts: # src/main/java/duke/task/TaskList.java

damithc and others added 12 commits July 23, 2020 23:27

Add Gradle support

3b19ba1

build.gradle: Update version to 8.29

a75fcee

Complete Level-1: Greet, Echo, Exit

77ee7e3

Complete Level-2: Add, List

27ef333

Complete Level-3: Mark as Done

cecd097

Complete Level-4: ToDo, Event, Deadline

cd80752

Complete A-TextUiTesting: Automated Text UI Testing

6c6269a

Complete Level-4: ToDo, Events, Deadlines (updated)

96e15e5

Completed A-TextUiTesting: Automated Text UI Testing (updated)

76ba712

Complete A-TextUiTesting: Automated Text UI Testing (updated new)

6597495

Complete Level-5: Handle Errors

cfcf682

Complete Level-6: Delete

e59f9f4

vvan-essa reviewed Feb 1, 2021

View reviewed changes

markuz5116 reviewed Feb 2, 2021

View reviewed changes

NiniJiaying reviewed Feb 2, 2021

View reviewed changes

yanlingkuek added 15 commits February 5, 2021 16:30

Reorganise Level 1-6

5fcc0a6

Correct style issues

9a16853

Add JavaDoc

89416ab

Add saving feature (Complete Level 7)

dbb3271

Add features to read date and time (Complete Level 8)

e60caaa

Add JavaDoc and user instructions

0b06a57

Merge branch 'branch-Level-8'

b2ed9a0

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

Add classes Ui, Storage, Parser, Command, AddCommand, DeleteCommand, …

628dd4f

…ExitCommand, DoneCommand, TaskdateCommand (Complete A-MoreOOP)

Divide classes into packages

e0eb53a

Add JUnit tests (Complete A-JUnit)

96fe041

Add more JavaDocs (Complete A-JavaDoc)

3860020

Tweak code to comply with coding standard (Complete A-CodingStandard)

1213091

Fix duke.txt file reading issues

47369e7

Add new command: find (Complete Level-9)

38eb8b5

Merge branch 'branch-A-CodingStandard'

7390e40

yanlingkuek and others added 17 commits February 7, 2021 11:40

Merge branch 'branch-Level-9'

cd44e0f

# Conflicts: # src/main/java/duke/Storage.java # src/main/java/duke/task/TaskList.java

Merge branch 'add-gradle-support' of https://github.com/yanlingkuek/ip

573bb90

Use checkstyle (Complete A-CheckStyle)

177e775

Add GUI (Complete Level-10)

17ca5e2

Fix issue with done command

a70beff

Add assertions (Complete A-Assertions)

699f6cc

Improve code quality (Complete A-CodeQuality)

1e9c795

Add feature to check if the new task to be added already exists in th…

ff944f3

…e current task list. (Complete BCD-Extension, C-DetectDuplicates)

Merge pull request #2 from yanlingkuek/branch-A-Assertions

189ae2f

Add assertions (Complete A-Assertions)

Merge branch 'master' of https://github.com/yanlingkuek/ip into branc…

d7976a0

…h-BCD-Extension # Conflicts: # data/duke.txt

Merge branch 'branch-A-CodeQuality'

5c8e8f0

# Conflicts: # data/duke.txt # src/main/java/duke/Duke.java # src/main/java/duke/task/TaskList.java

Merge branch 'branch-BCD-Extension'

51a933e

# Conflicts: # src/main/java/duke/task/TaskList.java

Change exception message

2b78ce8

Add screenshot of UI and window title.

9b21785

Update README file.

67d669d

Add user guide and update README.

fc9f60a

Change main class name.

ec49759


		protected String deadline;

		public DeadlineTask(String description, String deadlline) {

Conversation

yanlingkuek commented Jan 31, 2021

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!

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

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