[Tai Kah Kiang] iP by kktai1512 · Pull Request #70 · nus-cs2113-AY2122S2/ip

kktai1512 · 2022-02-02T08:51:29Z

Completed until Week 3 (Level 3, then A- Coding Standard)

Has greetings, todo, and toggle complete

yanjie1017

Great work in overall! Levels of abstraction are present too.

yanjie1017 · 2022-02-02T22:57:53Z

+    }
+
+    //main
+    public static void run (String line) {


The title of this method can be more informative.

yanjie1017 · 2022-02-02T23:03:10Z

+        String addInfo = words.size() >= 2 ? words.get(1) : null;
+
+        switch (type) {
+        case "todo":


Can consider extracting out some methods or sub-methods to a new class (eg. TaskManager, TaskList) to manage any functions related to Task

cczhouqi

Overall, I think your code is clear and easy to follow👍🏻 There are only some minor formatting things that you may want to improve on.

cczhouqi · 2022-02-03T03:38:35Z

+    }
+
+    //command methods
+    public static void hello() {


I like how you abstracted the hello message (and other tasks), that makes your main method clean and clear :D

cczhouqi · 2022-02-03T03:40:25Z

+    public static void run (String line) {
+        ArrayList<String> words = processInput(line, " ");
+        String command = words.get(0);
+        String description = words.size()>= 2 ? words.get(1) : null;


Format can be more consistent here: you may want to add a space between “word.size()” and “>=“.

cczhouqi · 2022-02-03T03:41:05Z

+
+        hello();
+
+        Scanner sc = new Scanner (System.in);


Maybe the name “sc” can be more informative.

cczhouqi · 2022-02-03T03:43:04Z

+
+    public Event(String name, String time) {
+        super(name);
+        this.time= time;


Same as in Duke.java line 94, you may want to add a space between “this.time” and “=“ to make it more consistent.

khseah

Overall clean and easy to read. However I feel the code can be more consistent as some methods and blocks of code are separated by a new line while others are not.

khseah · 2022-02-03T03:38:48Z

+
+    public Deadline(String name, String deadline) {
+        super(name);
+        this.deadline= deadline;


Suggested change

this.deadline= deadline;

this.deadline = deadline;

Maybe add a space to improve readability. I noticed this in other parts of the code as well

khseah · 2022-02-03T03:39:26Z

+
+    @Override
+    public String toString() {
+        return "[D]" + super.toString() + String.format("(%s)", this.deadline) ;


Suggested change

return "[D]" + super.toString() + String.format("(%s)", this.deadline) ;

return "[D]" + super.toString() + String.format("(%s)", this.deadline);

Not sure why there is a space before ;

khseah · 2022-02-03T03:47:18Z

+        if (divider != -1) {
+            words.set(0, line.substring(0, divider));
+            words.add(line.substring(divider+1));


Try and avoid magic literals

khseah · 2022-02-03T03:49:59Z

+        printWithDivider(task.toString());
+    }
+
+    //main


This comment isn't really helpful. Maybe provide more details?

khseah · 2022-02-03T03:52:26Z

@@ -0,0 +1,14 @@
+public class Event extends Task{
+


This class has a empty line before its attributes. However I noticed other classes do not have this empty line. Try and make it more consistent

Merging from level 5, no ff

stephlewyh · 2022-02-21T10:38:26Z

 public class Duke {
-    public static void main(String[] args) {
+
+    //fields


This comment is not very helpful in providing further explanation on how the variable is used. It can be omitted.

stephlewyh · 2022-02-21T10:40:15Z

+    public static void run(String line) throws DukeException {
+        ArrayList<String> words = IOMethods.splitToTwo(line, " ");
+        String command = words.get(0);
+        String description = words.size()>= 2 ? words.get(1) : null;


2 is a magic number that you could avoid. Consider using named constants instead.

stephlewyh · 2022-02-21T10:41:16Z

+
+

Remove empty lines like these to improve code readability.

stephlewyh · 2022-02-21T10:42:41Z

+        String taskType = input.substring(indexOfSpace + 2, indexOfSpace+3);
+        String status = input.substring(indexOfSpace + 5, indexOfSpace + 6);
+        String nameAndDate = input.substring(indexOfSpace+ 7);


There are many magic numbers here which you can avoid.

stephlewyh · 2022-02-21T10:44:06Z

+        System.out.println(breakLine);
+    }
+
+    public static void errorHandler(String input) throws DukeException {


This is method is quite long (>30 LoC). Consider applying SLAP for each of the commands for further abstraction.

stephlewyh · 2022-02-21T10:49:38Z

+        case "todo":
+        case "event":
+        case "deadline":


You can refer to switch case statement layout in the Java coding standards and add fallthrough comments here.

stephlewyh · 2022-02-21T10:57:43Z

+                if (command.equals("event") || command.equals("deadline")) {
+                    int indexOfSlash = input.indexOf("/");
+                    String date = indexOfSlash == -1 ? "" : input.substring(indexOfSlash);
+                    if (date.length() <= 1) {
+                        String errorMsg = String.format("%s requires a valid date in the format taskName /date\n", command);
+                        throw new DukeException(errorMsg);
+                    }
+                }
+            }


It is strange that event and deadline blocks of code are handled here after the fallthrough and there seems to be no course of action for todo beside checking for empty taskName. These lines seems trippy and you could try to restructure it to a simple form.

Branch level 8

Level 9

A-JavaDocs

kktai1512 added 7 commits January 21, 2022 17:13

add greeting

e1b96e2

refactored level_0

9a88290

add greet echo exit, and abstracted out print with divider

f35f63d

add list ability using arraylist

ca00879

add toggle complete

212f360

add todo, deadline and event subclass (level 4)

47cce8f

add automated testing

85f8d73

kktai1512 force-pushed the master branch 2 times, most recently from 7d1cef2 to e86117d Compare February 2, 2022 12:31

abstract run function from main, clean up code, fix event toString typo

bebd84d

kktai1512 force-pushed the master branch from e86117d to bebd84d Compare February 2, 2022 12:32

yanjie1017 reviewed Feb 2, 2022

View reviewed changes

cczhouqi reviewed Feb 3, 2022

View reviewed changes

khseah reviewed Feb 3, 2022

View reviewed changes

kktai1512 added 10 commits February 8, 2022 23:42

abstract out task related things to TaskManager class

66cb637

add error handling

ff54efb

add DukeException class

1bfe6b6

Merge branch 'branch-Level-5'

1da8e96

Merging from level 5, no ff

add to packages

2e14be2

Merge branch 'add-package'

38c3895

add delete task function

0a80df8

add file writer and reader

4732cce

complete filereader

676c634

merge level 7

76c50b1

stephlewyh reviewed Feb 21, 2022

View reviewed changes

kktai1512 added 5 commits March 2, 2022 04:01

add more oop- parser, ui, command, storage

63ce6a0

change deadline of Deadline to by, time of Event to on

f64e5e9

add date/time

6fdb78a

add find functionality

f304d13

add date and time

318d046

kktai1512 and others added 19 commits March 2, 2022 16:15

Merge pull request #1 from kktai1512/branch-Level-8

c9546db

Branch level 8

merge from master

4c0e6d8

Merge pull request #2 from kktai1512/branch-Level-9

b6adc88

Level 9

add javadoc for taskmanager and storage

921bb7f

Update README.md

550f812

add more javadocs

04944b6

change to autosave everytine tasks has changed

b7d0ed6

change such that task doesnt accept nameless / wrong name

b1e1f7e

check for style

52e2312

check for style

05026dc

remove checkstyle

d4a94ac

Merge pull request #3 from kktai1512/branch-A-JavaDoc

8579519

A-JavaDocs

allow non alphanumeric name, disallow / in taskName

d6bef1e

allow non alphanumeric name, disallow / in taskName

424ec24

Update README.md

75b13f8

change readme.md

816230c

change readme.md

7ffb9af

Update README.md

b2c7657

Update README.md

c170780

	return "[D]" + super.toString() + String.format("(%s)", this.deadline) ;
	return "[D]" + super.toString() + String.format("(%s)", this.deadline);

Uh oh!

Conversation

kktai1512 commented Feb 2, 2022

Uh oh!

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

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

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

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants