[Ma Yitao] ip by PrinceCatt · Pull Request #101 · nus-cs2113-AY2425S1/ip

PrinceCatt · 2024-09-03T06:30:31Z

No description provided.

Toby-Yu

Looks good to me

Toby-Yu · 2024-09-05T02:38:30Z

+            type = messages.get(i-1).getType();
+            String typeSign = "";
+            String startTime = "";
+            String endTime = "";
+            String By = "";
+            String From = "";
+            if(type == 1){
+                typeSign = "[T]";
+            }
+            else if(type == 2){
+                By = " By: ";
+                typeSign = "[D]";
+                endTime = messages.get(i-1).getEndTime();
+            }
+            else if(type == 3){
+                typeSign = "[E]";
+                By = " By: ";
+                From = " From: ";
+                startTime = messages.get(i-1).getStartTime();
+                endTime = messages.get(i-1).getEndTime();
+            }


maybe using case is also another way.

Toby-Yu · 2024-09-05T02:41:37Z

-        System.out.println("-----------------------------------");
-        System.out.println("added:" + message.getMessage());
-        System.out.println("-----------------------------------");
+    public static void addList(messageList list, String input){


it looks good to me in this addList function

Toby-Yu · 2024-09-05T02:45:18Z

+    public Message(String message, String startTime, String endTime, int type) {
+        this.message = message;
+        this.isDone = false;
+        this.startTime = startTime;
+        this.endTime = endTime;
+        this.type = type;
+    }


the variable name follows the coding standard which is good.

Toby-Yu · 2024-09-05T02:45:52Z

+            String typeSign = "";
+            String startTime = "";
+            String endTime = "";
+            String By = "";
+            String From = "";


the variable name is easy for me to know the use of it

LuxLucky7

The code works well but readability could be worked on

LuxLucky7 · 2024-09-05T03:49:22Z

    private String endTime;

+    //todo
+    public Message(String message) {


Would the name of the method as a verb give a better idea on what the method does?

LuxLucky7 · 2024-09-05T03:50:59Z

+    }
+
+    //event
+    public Message(String message, String startTime, String endTime, int type) {


Maybe the use and reason for overloading the Message function could be made more explicit?

LuxLucky7 · 2024-09-05T03:55:53Z

@@ -57,19 +57,72 @@ public static void listShow(messageList list) {
            else {
                doneSign = "";
            }


Could the overuse of the word message and its variations - as variable name, class name, method name - get confusing for the writer and the reader? Maybe you could try using other names

LuxLucky7 · 2024-09-05T03:58:23Z

+            List<Message> messages = list.getMessages();
+            Message message = new Message(input);
+            String[] strings = input.split(" ");
+            String eventName = strings[1].split(" ")[1];
+            messages.add(message);
+            list.setMessages(messages);
+            System.out.println("-----------------------------------");
+            System.out.println("added:" + message.getMessage());
+        }
+
+        else if(input.contains("deadline")) {
+            List<Message> messages = list.getMessages();
+            String[] strings = input.split("/");
+            String endTime = strings[strings.length-1].split(" ")[1];
+            String eventName = strings[strings.length-2].split(" ")[1];
+            Message message = new Message(eventName, endTime, 2);
+            messages.add(message);
+            list.setMessages(messages);
+            System.out.println("-----------------------------------");
+            System.out.println("added:" + message.getMessage());
+        }
+
+        else if(input.contains("event")) {
+            List<Message> messages = list.getMessages();
+            String[] strings = input.split("/");


Would a function, which does the same task that you are doing now, within each if-else if bracket make it more readable and clear?

TJW0911 · 2024-09-18T07:36:06Z

+        String logo = "__   __         _        \n"
+                + "\\ \\ / /_     _ / /___ __ ______ ______      \n"
+                + " \\   / //  / // ___/ / // ___ ///--/ /     \n"
+                + "  / // /__/ //   \\  / // / / ///__/ /  \n"
+                + " /_/ /____/ /_/ \\_\\/_//_/ /_//_____/ \n";
+        System.out.println(logo);
+        System.out.println("Hello, I am Yukino!\n");
+        System.out.println("What can I do for you?\n");
+        System.out.println("-----------------------------------\n");


Maybe this can be refactored into a method such as printWelcomeMessage() so as to make main() more concise and readable.

TJW0911 · 2024-09-18T07:44:34Z

+import java.util.ArrayList;
+import java.util.List;
+
+public class messageList {


Generally class names should be in Pascal case according to the coding standard so this should be MessageList. See https://se-education.org/guides/conventions/java/basic.html

TJW0911 · 2024-09-18T07:54:30Z

+    public static void mark(messageList list, String input) {
+        List<Message> messages = list.getMessages();
+        String[] sentences = input.split(" ");
+        int number = Integer.parseInt(sentences[1]);
+        if(number > messages.size()) {
+            System.out.println("Sorry, you are marking an event that has not been added");
+            return;
+        }
+
+        if(input.contains("unmark")){
+            messages.get(number-1).setDone(false);
+            System.out.println("-----------------------------------\n");
+            System.out.println("I have marked this task as not done!\n" + "[X] " + messages.get(number-1).getMessage());
+            System.out.println("-----------------------------------\n");
+        }
+        else {
+            messages.get(number - 1 ).setDone(true);
+            System.out.println("-----------------------------------\n");
+            System.out.println("I have marked this task as done!\n" + "[X] " + messages.get(number-1).getMessage());
+            System.out.println("-----------------------------------\n");
+        }


Maybe using a more descriptive name such as markMessage might be a better here. Additionally, you could consider using a separate method for mark and unmark to make your code more concise and readable.

TJW0911 · 2024-09-18T07:57:06Z

+        System.out.println("-----------------------------------\n");
+    }
+
+    public static void listShow(messageList list) {


Maybe using a more descriptive name such as listMessages is better here to describe the entity being listed.

TJW0911 · 2024-09-18T07:59:41Z

+        System.out.println("-----------------------------------\n");
+    }
+
+    public static void addList(messageList list, String input){


Maybe naming it addToList might be clearer here as you are adding the messages to a list and not adding a list of messages. You could also consider addMessage as well.

TJW0911 · 2024-09-18T08:02:11Z

+            List<Message> messages = list.getMessages();
+            Message message = new Message(input);
+            String[] strings = input.split(" ");
+            String eventName = strings[1].split(" ")[1];
+            messages.add(message);
+            list.setMessages(messages);
+            System.out.println("-----------------------------------");
+            System.out.println("added:" + message.getMessage());
+        }
+
+        else if(input.contains("deadline")) {
+            List<Message> messages = list.getMessages();
+            String[] strings = input.split("/");
+            String endTime = strings[strings.length-1].split(" ")[1];
+            String eventName = strings[strings.length-2].split(" ")[1];
+            Message message = new Message(eventName, endTime, 2);
+            messages.add(message);
+            list.setMessages(messages);
+            System.out.println("-----------------------------------");
+            System.out.println("added:" + message.getMessage());
+        }
+
+        else if(input.contains("event")) {
+            List<Message> messages = list.getMessages();
+            String[] strings = input.split("/");
+            String startTime = strings[strings.length-2].split(" ")[1];
+            String endTime = strings[strings.length-1].split(" ")[1];
+            String eventName = strings[strings.length-3].split(" ")[1];
+            Message message = new Message(eventName, startTime, endTime, 3);
+            messages.add(message);
+            list.setMessages(messages);
+            System.out.println("-----------------------------------");
+            System.out.println("added:" + message.getMessage());


Perhaps you could refactor these 3 parts into methods to make this part more concise and readable.

TJW0911 · 2024-09-18T08:04:43Z

@@ -0,0 +1,76 @@
+package Entity;
+
+public class Message {


Maybe you could consider a different name for this class that represents the tasks to be stored as I initially thought this meant the input messages instead of the tasks that are being created and stored in the IP.

TJW0911 · 2024-09-18T08:07:25Z

+        this.messages = messages;
+    }
+
+    public static void add(Message message) {


Perhaps naming this as addMessage might make it clearer and more consistent with the other methods above.

TJW0911 · 2024-09-18T08:08:14Z

+
+public class messageHandler {
+
+    public static void preHandle(messageList list) {


Maybe using a more descriptive name such as preHandleInput might be better here to specify what this method is doing.

TJW0911 · 2024-09-18T08:09:50Z

+
+        Scanner scanner = new Scanner(System.in);
+        String input = null;
+        messageList list = new messageList();


Maybe naming this as messageList or messages might be better to specify what this list contains.

…o branch-level-6

branch-level-8

branch-level-9

PrinceCatt added 8 commits August 29, 2024 00:26

week2

b4ad2f6

level1_echo

07e9994

level2_add_list

6b08004

level3_mark_done

844d22f

change_logo

7cf6453

handler_in_progress

d188478

Merge branch 'master' of https://github.com/PrinceCatt/ip

b14f78b

level4_todo_event_deadline

ad00caa

Toby-Yu reviewed Sep 5, 2024

View reviewed changes

LuxLucky7 reviewed Sep 5, 2024

View reviewed changes

level 5

f9cc828

TJW0911 reviewed Sep 18, 2024

View reviewed changes

PrinceCatt and others added 18 commits September 19, 2024 10:14

level-6-save

b26d7fb

Merge branch 'branch-level-6' of https://github.com/PrinceCatt/ip int…

445823b

…o branch-level-6

saveNdelete

0e4970e

Merge branch 'master' of https://github.com/PrinceCatt/ip

f06296d

Merge branch 'branch-level-6'

3f8a6dd

Save_Delete_completed

e804b8f

branch-level-8

546f2f2

branch-level-9

423ae8a

branch-level-9

c068172

revert

8b306e3

Merge branch 'master' into branch-level-8

f28e85a

Merge pull request #1 from PrinceCatt/branch-level-8

c2b96a1

branch-level-8

Merge branch 'master' into branch-level-9

87fe59f

Merge pull request #2 from PrinceCatt/branch-level-9

b975c13

branch-level-9

Final update

b5be4ba

Final merge

da9f1bf

Merge branch 'master' of https://github.com/PrinceCatt/ip

c2bb4b2

typo

5f04586

Update README.md

4bcc0a1


		public class messageHandler {

		public static void preHandle(messageList list) {

Conversation

PrinceCatt commented Sep 3, 2024

Uh oh!

Toby-Yu 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!

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

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants