[Kairos Koh] iP by kairoskoh · Pull Request #189 · nus-cs2113-AY2122S1/ip

kairoskoh · 2021-09-01T08:50:09Z

No description provided.

Add coding standards

Add Event.java Add ToDo.java

lamkwjonathan

Overall quite nicely done. Just watch out for camelCase when naming methods.

lamkwjonathan · 2021-09-02T04:31:25Z

src/main/java/Duke.java

+        String input;
+        Scanner in = new Scanner(System.in);
+        int counter = 0;
+        Task[] list = new Task[100];


Perhaps 'tasks' is a more appropriate name, since it is plural.

lamkwjonathan · 2021-09-02T04:35:49Z

src/main/java/Duke.java

+        PrintGoodbyeMessage();
+    }
+
+    private static void ActiveChat() {


I think 'ActiveChat' should be 'activeChat' instead, since methods should be in camelCase.

lamkwjonathan · 2021-09-02T04:38:04Z

src/main/java/Duke.java

+        }
+    }
+
+    private static void CheckIsEmptyList(Task[] list) {


'checkIsEmptyList' for 'CheckIsEmptyList', or perhaps even just 'isEmptyList' for boolean naming.
Similar camelCase method naming issues in other places as well.

lamkwjonathan · 2021-09-02T04:40:47Z

src/main/java/Duke.java

+        System.out.println("--------------------");
+        System.out.println("Got it. I've added this task: ");
+        System.out.println("[" + task.getLetter() + "] "
+                + "[" + task.getStatusIcon() + "] "


nice use of wrapped lines!

justinfidelis · 2021-09-02T05:58:18Z

src/main/java/Task.java

+        this.description = description;
+        this.isDone = false;
+    }
+


Perhaps it will be cleaner to override the .toString() method to return the entire [T][ ] task name string, instead of needing separate public getLetter() and getStatusIcon() methods.

public String toString() { String checkbox = isCompleted ? "[X]" : "[ ]"; return checkbox + " " + taskName; }

It is also possible to further override the function in the subclasses.
Example for Deadline subclass

@Override public String toString() { String checkbox = isCompleted ? "[X]" : "[ ]"; return "[D]" + super.toString() + " " + "(by: " + by + ")"; }

justinfidelis · 2021-09-02T07:20:59Z

src/main/java/Duke.java

+    }
+
+    private static int ProcessTasks(String input, int counter, Task[] list) {
+        if (input.contains("todo")) {


It might be cleaner to replace the strings "todo", "deadline", etc. with named constants of type static final String

justinfidelis · 2021-09-02T07:28:15Z

src/main/java/Duke.java

+        return counter;
+    }
+
+    private static void PrintDoneMessage(Task[] list, int itemNum) {


Nice use of specific PrintMessage methods to keep the overall code cleaner and easier to understand

justinfidelis · 2021-09-02T07:32:27Z

src/main/java/Duke.java

+    private static void PrintDoneMessage(Task[] list, int itemNum) {
+        System.out.println("--------------------");
+        System.out.println("Nice! I've marked this task as done:");
+        System.out.println( itemNum + ".[" + list[itemNum - 1].getStatusIcon() + "] " + list[itemNum - 1].getDescription() );


Linked to the comment on overriding the .toString() method in Task. By doing so, this part of code can be simplified to

System.out.println( itemNum + "." + list[itemNum - 1])

justinfidelis · 2021-09-02T07:34:55Z

src/main/java/Duke.java

+            int donePos = input.indexOf("/"); //finds pos of '/'
+            String description = input.substring(9,donePos);
+            String date = input.substring(donePos + 4);
+            Deadline newTask = new Deadline(description,date);
+            list[counter] = newTask;
+            counter += 1;
+            PrintAddedTaskMessage(newTask, counter);


It might be cleaner to refactor this into a separate method.
One way is to have additional methods in the Duke class that handles the different Task subclasses.
Another is to create a separate class TaskManager that stores the array of Tasks and has functions to add the different Task subclasses to keep the main Duke class clean. (I think this will be the best approach)
Another is to overload the subclass constructors to take in String[] or a similar data structure.

Add exceptions Improve code quality

ongweisheng

Generally well done, do take note if comments are unnecessary do remove them and see if you can abstract more of your code

ongweisheng · 2021-09-15T06:26:30Z

src/main/java/duke/Duke.java

+        // Welcome Message
+        printWelcomeMessage();
+
+        // Active Chat
+        activeChat();
+
+        // Goodbye Message
+        printGoodbyeMessage();
+    }


Note if the comments are necessary, else remove them if they are not

ongweisheng · 2021-09-15T06:27:29Z

src/main/java/duke/Duke.java

+    private static void activeChat() {
+        boolean isBye = false;
+        String input;
+        Scanner in = new Scanner(System.in);
+        int counter = 0;
+        Task[] list = new Task[100];
+
+        while(!isBye){
+            //store input
+            input = in.nextLine();
+            //process input
+            if (input.equals("bye")){ //check if bye
+                isBye = true;
+            } else if (input.equals("list")) { //check if list
+                processList(counter, list);
+            } else if (input.contains("done") ) { //check if done
+                processDone(input, list);
+            } else {
+                try {
+                    counter = processTasks(input, counter, list); //process tasks
+                } catch ( IllegalTaskException e ) {
+                    System.out.println("Please include /by for deadline and /at for event");
+                }
+            }
+        }
+    }


Consider implementing a Parser class to parse user inputs instead of putting everything in main/Duke

ongweisheng · 2021-09-15T06:29:18Z

src/main/java/duke/Duke.java

+    private static void processList(int counter, Task[] list) {
+        if (list[0] != null){
+            printListMessage(counter, list);
+        } else {
+            printListButEmptyMessage();
+        }
+    }
+
+    private static void processDone(String input, Task[] list) {
+        int donePos = input.indexOf("done");
+        if (list[0] != null) {
+            if (input.length() < donePos + 5) {
+                printDoneButNotSpecificMessage(); //when input contains done but no number
+            } else {
+                processValidDone(input, list, donePos); //when input contains done and specified number
+            }
+        } else {
+            printDoneButEmptyMessage(); //when input contains done but list is empty
+        }
+    }
+
+    private static void processValidDone(String input, Task[] list, int donePos) {
+        String itemNumDone = input.substring(donePos + 5, donePos + 6);
+        int itemNum = Integer.parseInt(itemNumDone);
+        list[itemNum - 1].setDone();
+        printDoneMessage(list, itemNum);
+    }
+
+    private static int processTasks(String input, int counter, Task[] list) throws IllegalTaskException{
+        if ( ( input.contains("deadline") || input.contains("event") ) && !input.contains("/")){
+            throw new IllegalTaskException();
+        }
+        if (input.contains("todo")) {
+            String description = input.substring(5);
+            ToDo newTask = new ToDo(description);
+            list[counter] = newTask;
+            counter += 1;
+            printAddedTaskMessage(newTask, counter);
+        } else if (input.contains("deadline")) {
+            int donePos = input.indexOf("/"); //finds pos of '/'
+            String description = input.substring(9,donePos);
+            if (!input.substring(donePos + 1, donePos + 3).equals("by")) {
+                throw new IllegalTaskException();
+            }
+            String date = input.substring(donePos + 4);
+            Deadline newTask = new Deadline(description,date);
+            list[counter] = newTask;
+            counter += 1;
+            printAddedTaskMessage(newTask, counter);
+        } else if (input.contains("event")) {
+            int donePos = input.indexOf("/"); //finds pos of '/'
+            String description = input.substring(6,donePos);
+            if (!input.substring(donePos + 1, donePos + 3).equals("by")) {
+                throw new IllegalTaskException();
+            }
+            String date = input.substring(donePos + 4);
+            Event newTask = new Event(description,date);
+            list[counter] = newTask;
+            counter += 1;
+            printAddedTaskMessage(newTask, counter);
+        } else {
+            System.out.println("Please specify tasks: todo, deadline or event");
+            System.out.println("Example - type in the following: todo read book");
+        }
+        return counter;
+    }


Consider creating TaskManger class to handle task manipulation as well as storing their data for your task list

ongweisheng · 2021-09-15T06:30:22Z

src/main/java/duke/Duke.java

+    private static void printDoneMessage(Task[] list, int itemNum) {
+        System.out.println("--------------------");
+        System.out.println("Nice! I've marked this task as done:");
+        System.out.println( itemNum + ".[" + list[itemNum - 1].getStatusIcon() + "] " + list[itemNum - 1].getDescription() );
+        System.out.println("--------------------");
+    }
+
+    private static void printListMessage(int counter, Task[] list) {
+        System.out.println("--------------------");
+        System.out.println("Here are the tasks in your list:");
+        for(int i = 0; i < counter; i += 1){
+            printListOfTaskSubMessage(list[i], i);
+        }
+        System.out.println("--------------------");
+    }
+
+    private static void printListButEmptyMessage() {
+        System.out.println("--------------------");
+        System.out.println("List is empty. Time to get productive!");
+        System.out.println("--------------------");
+    }
+
+    private static void printDoneButNotSpecificMessage() {
+        System.out.println("Please specify which task is done.");
+    }
+
+    private static void printDoneButEmptyMessage() {
+        System.out.println("--------------------");
+        System.out.println("Unable to tick off list.");
+        System.out.println("List is empty. Time to get productive!");
+        System.out.println("--------------------");
+    }
+
+    private static void printAddedTaskMessage(Task task, int i) {
+        System.out.println("--------------------");
+        System.out.println("Got it. I've added this task: ");
+        System.out.println("[" + task.getLetter() + "] "
+                + "[" + task.getStatusIcon() + "] "
+                + task.getDescription()
+                + task.getDate() );
+        System.out.println("Now you have " + i + " tasks in the list.");
+        System.out.println("--------------------");
+    }
+
+    private static void printListOfTaskSubMessage(Task task, int i) {
+        System.out.println(i + 1
+                + ".[" + task.getLetter() + "] "
+                + "[" + task.getStatusIcon() + "] "
+                + task.getDescription()
+                + task.getDate() );
+    }
+
+    private static void printGoodbyeMessage() {
+        System.out.println("--------------------");
+        System.out.println("Bye. Hope to see you again soon!");
+        System.out.println(" ");
+        System.out.println("--------------------");
+    }
+
+    private static void printWelcomeMessage() {
+        String logo = " ____        _        \n"
+                + "|  _ \\ _   _| | _____ \n"
+                + "| | | | | | | |/ / _ \\\n"
+                + "| |_| | |_| |   <  __/\n"
+                + "|____/ \\__,_|_|\\_\\___|\n";
+        System.out.println("Hello from\n" + logo);
+
+        System.out.println("--------------------");
+        System.out.println("Hello! I'm Duke");
+        System.out.println("What can I do for you?");
+        System.out.println(" ");
+        System.out.println("--------------------");
+    }
+}


Consider creating a UI class to handle your print statements instead

ongweisheng · 2021-09-15T06:31:29Z

src/main/java/duke/Event.java

+package duke;
+
+public class Event extends Task{
+
+    protected final static char LETTER = 'E';
+    protected String date;
+
+    public Event(String description, String date) {
+        super(description);
+        this.date = date;
+    }
+
+    public String toString() {
+        return description;
+    }
+
+    public char getLetter() {
+        return LETTER;
+    }
+
+    public String getDate() {
+        return "(at: " + date + ")";
+    }
+
+
+}


Consider creating packages for your Event, Todo classes etc.

Add Level 7. Save

Add find functions

Add JavaDoc

Add Level 5

Add Level 6

Add Level 7

kairoskoh added 9 commits August 19, 2021 21:07

Add Level 0. Greet (greets user and exits)

98e53ab

Add Level 1. Greet, Echo, Exit

e0d5c4d

Add Level-2. Add, List

ca6b0fe

Add Task.java

13d9a37

Add Level 3. Mark as Done

9dcbc95

Add fixes to some exception inputs

c3d9af5

Add coding standards

Add Deadline.java

72f7593

Add Event.java Add ToDo.java

Add Level 4. ToDos, Events, Deadlines

1e289bf

Improve Code Quality

7b50777

lamkwjonathan reviewed Sep 2, 2021

View reviewed changes

justinfidelis reviewed Sep 2, 2021

View reviewed changes

kairoskoh added 2 commits September 8, 2021 22:11

Add IllegalTaskException.java

af71eb3

Add exceptions Improve code quality

Add duke package

6c57ecf

ongweisheng reviewed Sep 15, 2021

View reviewed changes

kairoskoh and others added 16 commits September 15, 2021 21:37

Add Level 6. Delete and A-Collections

e8d8f41

Add DukeTaskData.txt

11315c6

Add Level 7. Save

Add minor changes to error message

7c26728

Add minimal OOP classes

3cf54db

Make the code more OOP

15d6adf

Add file to ignore

57d09ac

Add function to find a task by searching for a keyword

eac25f9

Add JavaDoc

c41f3af

Add find functions

b8098ba

Merge pull request #1 from kairoskoh/branch-Level-9

dfb01a0

Add find functions

Add JavaDoc

972d782

Merge pull request #2 from kairoskoh/branch-A-JavaDoc

663b585

Add JavaDoc

Add Level 5

e41e1f4

Add Level 6

bc60895

Add Level 7

b2b063c

Merge pull request #3 from kairoskoh/branch-Level-5

253abe3

Add Level 5

kairoskoh and others added 6 commits September 29, 2021 15:43

Merge pull request #4 from kairoskoh/branch-Level-6

4780c7e

Add Level 6

Merge pull request #5 from kairoskoh/branch-Level-7

411c77a

Add Level 7

Add user guide

493ebcc

Add more JavaDoc

7db017f

Fix minor bug when saving tasks

2ac4093

Remove hard-coded file paths

0da697a

Conversation

kairoskoh commented Sep 1, 2021

Uh oh!

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

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

4 participants