[Teo Jia Rong] iP by tjiarong · Pull Request #73 · nus-cs2113-AY2122S2/ip

tjiarong · 2022-02-02T14:20:09Z

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

…mprovement" This reverts commit 542dcb6.

…de for improvement"" This reverts commit 41c8e9c.

Teanweijun

Overall the code looks nice. Just have a few misc improvements that could be made. Keep it up!

Teanweijun · 2022-02-03T02:38:21Z

+        String command = line.split(" ")[0];
+        while (!command.equals("bye")) {
+            switch (command) {
+            case "list":
+                printList(tasks);
+                break;
+            case "unmark":
+                unmarkTask(line, tasks);
+                break;
+            case "mark":
+                markTask(line, tasks);
+                break;
+            case "todo":
+                addToDo(line, tasks);
+                break;
+            case "deadline":
+                addDeadline(line, tasks);
+                break;
+            case "event":
+                addEvent(line, tasks);
+                break;
+            default:
+                addTask(line, tasks);
+                break;
+            }
+            line = in.nextLine();
+            command = line.split(" ")[0];
+        }


Consider extracting this section into a separate method? It could make your main more readable.

Teanweijun · 2022-02-03T02:40:20Z

-                + "|____/ \\__,_|_|\\_\\___|\n";
-        System.out.println("Hello from\n" + logo);
+        final int MAX_TASK = 100;
+        Task[] tasks = new Task[MAX_TASK];


tasks here is a global variable... Is there a reason you have to pass them explicitly into your methods such as unmarkTask?

Teanweijun · 2022-02-03T02:43:11Z

+                    + "| )      | (___) |   | |   | )   ( |   | |   | (___) || )  \\  | \n"
+                    + "|/       (_______)   )_(   |/     \\|   )_(   (_______)|/    )_) \n";
+
+        System.out.println("_____________________________________________\n"


Since you use this line a lot, you could consider creating a static String to print this line instead of copying it all the time?

Teanweijun · 2022-02-03T02:44:59Z

+
+    @Override
+    public String toString() {
+        return ("[D]" + super.toString() + " (at: " + getPeriod() + ")");


This method exists inside the Events class already. It might be better if you printed period instead of calling getPeriod. Similar improvements can be made throughout the code.

Teanweijun · 2022-02-03T02:49:15Z

+    }
+
+    private static void addEvent(String line, Task[] tasks) {
+        String arg = line.split(" ", 2)[1];


This line.split by space is repeated throughout your adds. I think it'd be better if you could handle this before passing it into the function!

Teanweijun · 2022-02-03T02:50:49Z

+        System.out.println("_____________________________________________\n"
+                + "Got it. I've added this task: \n"
+                +  t + "\n"
+                + "Now you have " + (t.getTaskCount() - 1) + " tasks in the list." + "\n"
+                + "_____________________________________________\n"
+        );


I like how your print statements all end with "\n", makes it very readable!

XunyiiZ · 2022-02-03T03:46:13Z

+        String eventDate = arg.split("/at ", 2)[1];
+        Task t = new Events(event, eventDate);
+        tasks[t.getTaskCount() - 1] = t;
+        System.out.println("_____________________________________________\n"


Maybe can make the line a constant variable since you use it a lot.

XunyiiZ · 2022-02-03T03:51:46Z

+        String logo = "_______  _______ _________ _______ _________ _______  _\n"
+                    + "(  ____ )(  ___  )\\__   __/(  ___  )\\__   __/(  ___  )( (    /|\n"
+                    + "| (    )|| (   ) |   ) (   | (   ) |   ) (   | (   ) ||  \\  ( |\\\n"
+                    + "| (____)|| |   | |   | |   | (___) |   | |   | |   | ||   \\ | |\n"
+                    + "|  _____)| |   | |   | |   |  ___  |   | |   | |   | || (\\ \\) | \n"
+                    + "| (      | |   | |   | |   | (   ) |   | |   | |   | || | \\   | \n"
+                    + "| )      | (___) |   | |   | )   ( |   | |   | (___) || )  \\  | \n"
+                    + "|/       (_______)   )_(   |/     \\|   )_(   (_______)|/    )_) \n";
+
+        System.out.println("_____________________________________________\n"
+                        + logo + "\n"
+                        + "Hello! I'm Duke\n"
+                        + "What can I do for you?\n"
+                        + "_____________________________________________\n"
+                );


it's really readable to change a new line when you add \n

XunyiiZ · 2022-02-03T03:55:38Z

+            System.out.println("_____________________________________________\n"
+                    + "OK, I've marked this task as not done yet:\n"
+                    + tasks[taskNum] + "\n"
+                    + "_____________________________________________\n"


since you have use this print method in many classes maybe can consider making it as a method that every class can use

XunyiiZ · 2022-02-03T04:03:42Z

+            case "list":
+                printList(tasks);
+                break;
+            case "unmark":


matching switch case coding standard!

… last week but did not tag and commit properly)

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

stephlewyh · 2022-02-22T08:23:19Z

+        String eventDate = arg.split(" \\(at: ", 2)[1].split("\\)")[0];
+        Task t = new Events(event, eventDate);
+        tasks.add(t);
+        System.out.println("_____________________________________________\n"


You have several println statements with line headers like this. How about shortening the print statements by declaring a header variable and reusing it?

stephlewyh · 2022-02-22T08:24:13Z

+    public static final String FILE_MISSING_MESSAGE = "File does not exist. Creating file.";
+    public static final String FILE_FOUND_MESSAGE = "File found. Loading file into database.";
+
+    public static void main(String[] args) throws IOException {


Excellent use of SLAP here.

stephlewyh · 2022-02-22T08:26:00Z

+        String arg = line.split("] ", 2)[1];
+        String task = arg.split(" \\(by: ", 2)[0];
+        String dueDate = arg.split(" \\(by: ", 2)[1].split("\\)")[0];


2 appears to be a magic number which you could avoid. How about trying named constants?

stephlewyh · 2022-02-22T08:27:30Z

+import java.util.ArrayList;
+import java.util.Scanner;
+
+public class Potaton {


You have little or no javadoc comments. Including them in this class would greatly help the reader quickly understand what some of the methods are doing.

stephlewyh · 2022-02-22T08:28:27Z

+                try {
+                    addToDo(line, tasks);
+                } catch (IndexOutOfBoundsException e) {
+                    // Refactor this later
+                    System.out.println("_____________________________________________\n"
+                            + "Error: The description of a todo cannot be empty." + "\n"
+                            + "_____________________________________________\n"
+                    );
+                }


This block of code could be refactored so as to ensure SLAP.

Level-8 and minor change

Level-9 and minor changes

Added A-Javadoc

tjiarong added 14 commits January 20, 2022 23:29

First edit (Test)

4c9f6d7

Level 0 Greet

ade8fe6

Added Duke Level 1

803dcf2

Added Duke level 2

ac02ab7

Added Duke level 3

38ef2a2

Added Task class file

0eb20d8

Added A-CodingStandard

b113789

Updated tags and minor changes

1aacf1e

Completed Level 4

c182824

Added Duke A-CodeQuality, will continue to examine code for improvement

542dcb6

Revert "Added Duke A-CodeQuality, will continue to examine code for i…

41c8e9c

…mprovement" This reverts commit 542dcb6.

Revert "Revert "Added Duke A-CodeQuality, will continue to examine co…

dc57c00

…de for improvement"" This reverts commit 41c8e9c.

Added Duke A-CodeQuality, will continue to examine code for improvement

91b5fd7

Added Duke A-TextUiTesting

b36c0ae

Teanweijun reviewed Feb 3, 2022

View reviewed changes

XunyiiZ reviewed Feb 3, 2022

View reviewed changes

tjiarong added 13 commits February 10, 2022 01:37

Changed to a new branch

3a86bf3

Added Duke level-5

6b53c72

Merge branch 'branch-Level-5'

fd7bd0c

Increment A-Packages

ef38854

Added A-Exceptions and minor changes

bc497f4

Minor updates (Also tagged and updated A-Exception wrongly, completed…

bfa6e64

… last week but did not tag and commit properly)

Added branch-Level 6

15e16ff

Added branch-Level-7

b76374c

Merge branch 'branch-Level-6'

beb5bae

Resolve conflicts with branch-Level-7

561ac63

Merge branch 'branch-Level-7'

22cfb37

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

Added .gitignore

671ab74

Minor bug fixes

d4d5026

Minor changes

8f173c2

stephlewyh reviewed Feb 22, 2022

View reviewed changes

tjiarong and others added 15 commits March 3, 2022 23:21

Added Level-9

c3b312e

Added Level-8

29a70f3

Minor changes

af963b4

Level-8 and minor change

474644f

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

65497a8

Level-8 and minor change

Level-9 and minor changes

7393ea1

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

a86f54e

Level-9 and minor changes

Extracted classes

3c8da1b

Added A-Javadoc

45dd4e0

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

b3bea1c

Added A-Javadoc

Added A-UserGuide

1d308c3

Added A-MoreOOP

bb89e9f

Minor Updates

3b9d404

Hello

ddf88a6

Goodnight

ae7caeb

Uh oh!

Conversation

tjiarong commented Feb 2, 2022

Uh oh!

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

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