[TrungBui32] iP by TrungBui32 · Pull Request #112 · nus-cs2113-AY2425S1/ip

TrungBui32 · 2024-09-04T05:25:45Z

No description provided.

Yisoong

Looks good overall but you might want to rename some of your variables for greater clarity. Good Job !!

Yisoong · 2024-09-04T06:30:09Z

+    private static int last_index = 0;
+
+    public static void echo(String msg) {
+        System.out.println("____________________________________________________________\n");


I noticed that this was repeated many times. Perhaps it would be good to use a utility function to print the horizontal lines instead.

Yisoong · 2024-09-04T06:37:49Z

+
+public class Poirot {
+    private static Task[] list_actions = new Task[100];
+    private static int last_index = 0;


Perhaps it would be better to use lastIndex instead of last_index.

Yisoong · 2024-09-04T06:40:31Z

+        System.out.println("Hello! I'm POIROT\n");
+        System.out.println("What can I do for you?");
+        System.out.println("____________________________________________________________\n");
+        boolean working = true;


Since this is a boolean, it might be better to call it "isWorking".

Yisoong · 2024-09-04T06:43:54Z

+        System.out.println("____________________________________________________________\n");
+    }
+
+    public static void add(Task action) {


"add" is a bit ambiguous. It might be better to use "addTask" instead.

TPH777

Coding standards and code quality is not strictly followed, improvements can be made

TPH777 · 2024-09-04T06:46:04Z

+    private static int last_index = 0;
+
+    public static void echo(String msg) {
+        System.out.println("____________________________________________________________\n");


Abstracting the printing of divider to a function will improve readability

TPH777 · 2024-09-04T06:46:40Z

+
+public class Poirot {
+    private static Task[] list_actions = new Task[100];
+    private static int last_index = 0;


size may be a better name than last_index

TPH777 · 2024-09-04T06:47:31Z

+import java.util.Scanner;
+
+public class Poirot {
+    private static Task[] list_actions = new Task[100];


Use camelCase for variables, E.g. listActions, but a better name will be tasks

TPH777 · 2024-09-04T06:49:18Z

+        } else {
+            System.out.println("____________________________________________________________\n");
+            for (int i = 0; i < last_index; i++) {
+                System.out.println((i + 1) + ".[" + list_actions[i].getStatusIcon() + "]" + list_actions[i].getDescription());


Abstract this to a function and give variable names for list_actions[i].getStatusIcon() and list_actions[i].getDescription to improve code readability

TPH777 · 2024-09-04T06:51:16Z

+                    System.out.println("____________________________________________________________\n");
+                    break;
+                case "unmark":
+                    int y = Integer.parseInt(list_input[1]) - 1;


Set a variable for list_input[1] as it is unclear what is that

TPH777 · 2024-09-04T06:52:00Z

+                    System.out.println("____________________________________________________________\n");
+                    break;
+                case "unmark":
+                    int y = Integer.parseInt(list_input[1]) - 1;


use a better name instead of y

Merge branch-Level-5 with master branch

Mahesh1772

Good job!

The code looks clean and solid. Try to make use of inheritance even more in your code as OOP make coding more scalable and easier to manage. Consider adding JavaDocs in the future commits

Mahesh1772 · 2024-09-20T08:00:54Z

+    }
+    @Override
+    public String toFileString() {
+        return "D | " + (isDone ? "1" : "0") + " | " + description + " | " + by;


Consider adding the description in the parent classes toFIleString() to make full use of the inheritance principles

Mahesh1772 · 2024-09-20T08:01:09Z

+
+    @Override
+    public String toString() {
+        return "[E][" + getStatusIcon() + "] " + description + " (from: " + from + " to: " + to + ")";


Consider adding the description in the parent classes toFIleString() to make full use of the inheritance principles

Mahesh1772 · 2024-09-20T08:02:00Z

+    public static final String LINE = "____________________________________________________________\n";
+    private static Task[] tasks = new Task[100];
+    private static final String FILE_PATH = "./data/duke.txt";
+    private static int last_index = 0;


Consider naming the variables with more than one word in camelCase

Mahesh1772 · 2024-09-20T08:03:35Z

@@ -0,0 +1,204 @@
+import java.io.*;


Try not to use the * to import everything. Instead explicitly import what is needed.

Mahesh1772 · 2024-09-20T08:06:28Z

+    public abstract String toString();
+    public abstract String toFileString();


Consider making then non abstract methods, as they can be easily utilized by sub-classes. As such, Task could not be a abstract class.

Mahesh1772 · 2024-09-20T08:09:46Z

+                        tasks[x].setDone(true);
+                        System.out.println(LINE);
+                        System.out.println("Nice! I've marked this task as done:\n");
+                        System.out.print("[" + tasks[x].getStatusIcon() + "] ");
+                        System.out.println(tasks[x].getDescription());
+                        System.out.println(LINE);
+                        saveTasksToFile(); // Save the task after marking it
+                        break;
+                    case "unmark":
+                        validateTaskNumber(list_input);
+                        int y = Integer.parseInt(list_input[1]) - 1;
+                        tasks[y].setDone(false);
+                        System.out.println(LINE);
+                        System.out.println("OK, I've marked this task as not done yet:\n");
+                        System.out.print("[" + tasks[y].getStatusIcon() + "] ");
+                        System.out.println(tasks[y].getDescription());
+                        System.out.println(LINE);
+                        saveTasksToFile();


Consider saving the string formatted with the current isDone in a separate method inside Task class to reduce duplicate lines of code

Mahesh1772 · 2024-09-20T08:10:52Z

+
+        boolean working = true;
+        Scanner scan = new Scanner(System.in);
+        while (working) {


Consider doing a saving changes to file at the end of each iteration, as such avoiding to call it every time something is changed, reduce duplicate lines

Branch level 9

TrungBui32 added 4 commits August 29, 2024 16:54

Level 0: Rename, Greet, Exit

cd60db6

Level 1: Echo

4814b4e

Level 2. Add, List

4f308d2

Level 3. Mark as Done

6f3e69b

Yisoong reviewed Sep 4, 2024

View reviewed changes

TPH777 reviewed Sep 4, 2024

View reviewed changes

TrungBui32 added 9 commits September 4, 2024 21:52

Level 4. ToDos, Events, Deadlines

8877247

Try testing

92fffd2

Level 5. Handle Errors

bf62b4d

Merge branch 'branch-Level-5'

bc68ba7

Merge branch-Level-5 with master branch

Level 6. Delete

b806d12

Level 7. Save

b4c70c2

Merge branch 'branch-Level-6'

8d4b8cf

Merge branch 'branch-Level-7'

70f037a

Add saveTasksToFile to delete in Level 7. Save

b1c114f

Mahesh1772 reviewed Sep 20, 2024

View reviewed changes

TrungBui32 and others added 12 commits October 1, 2024 22:40

Make the program more OOP

3aaa8d4

Compile OOP version

46e6bef

Level 8. Dates and Times

1b9df27

Level 9. Find

85c5f24

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

1103df7

Branch level 9

Fix bugs of Deadline and Event

5ce99e1

Add JavaDocs comments

134e8fe

Update name in README.md

2e236e9

Update all features in README.md

b855458

rebuild ip.jar

edee1a4

Update classes

42416ac

Fix bug in Deadline and reformat code

58e96c7

		public abstract String toString();
		public abstract String toFileString();

Conversation

TrungBui32 commented Sep 4, 2024

Uh oh!

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

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

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

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants