[hongyijie06] iP by hongyijie06 · Pull Request #179 · nus-cs2113-AY2324S2/ip

hongyijie06 · 2024-02-07T05:56:24Z

No description provided.

mihirheda02

Overall, I think the code is very easy to understand and read. I think just focus on cleaning up the code a bit 👌

mihirheda02 · 2024-02-09T01:47:59Z

src/main/java/Duke.java

+
+
+
+        while (!line.equals("bye")){


I tried to look through the coding standard to see if there should be a space. There wasn't anything that specifically mentioned it, but all the examples had a space. It might be worth making this change throughout your code to keep it consistent.

mihirheda02 · 2024-02-09T01:49:14Z

src/main/java/Duke.java

+
+            if (line.equals("list")){
+                for (int i = 0; i < index; i ++) {
+


I think you can clean up random newlines like this

mihirheda02 · 2024-02-09T01:50:44Z

src/main/java/Duke.java

+
+                    System.out.println((i + 1) + ". " + list[i]);
+                }
+            }else {


I think adding a space between the "}" and "else" will follow coding standard more and keep the style of your code more consistent

mihirheda02 · 2024-02-09T01:56:33Z

src/main/java/Duke.java

+        System.out.println("Hello! I'm apple");
+        System.out.println("What can I do for you?");
+
+       int index = 0;


I think there should be one more space here for a 4 tab space

mihirheda02 · 2024-02-09T02:02:36Z

src/main/java/Duke.java

+
+
+
+


Additionally, I think you can clean up these new lines

mihirheda02 · 2024-02-09T04:06:05Z

src/main/java/Duke.java

-                + "| |_| | |_| |   <  __/\n"
-                + "|____/ \\__,_|_|\\_\\___|\n";
-        System.out.println("Hello from\n" + logo);
+        String list[] = new String[100];


Do you think it would be more beneficial to have a more descriptive name like "tasks"?

That might be better! Thank you!

a-wild-chocolate

Good job! You follows the code rule and the code quality is high. Keep well in the following levels.

a-wild-chocolate · 2024-02-10T13:10:09Z

src/main/java/Duke.java

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


You can try to use Arraylist which is more convenient in java

a-wild-chocolate · 2024-02-10T13:11:55Z

src/main/java/Duke.java

+            }
+
+        }
+        System.out.println("Bye. Hope to see you again soon!");


you can try to use more methods e.g. sayBey() to make the program more extendable for the future levels.

This reverts commit bedc8ec.

This reverts commit 4793c54.

nichyjt

Hi Yi Jie,

I think this is good work. There are various ways we can further stretch your code to adhere to better code quality principles, and I hope you consider them!

Throughout your program, there is heavy use of magic literals. This is not permissible by the course's coding quality metrics.
Some of your variable names could be named better (especially those single letter ones)
There is an opportunity to improve the code structure. Consider logically grouping your code by functionality and putting them into folders, rather than putting them into one file.
There is an opportunity to improve the level of abstraction in your program. Some functionality is repetitive and could be abstracted out in a function, which can help you make your codebase smaller and neater!

Try and work on my comments before the next tutorial!

Also for your consideration: There is a way to autoformat your code automatically every time you Ctrl-S on intellij which will save you much pain. :)

nichyjt · 2024-02-23T10:00:41Z

src/main/java/CreateFile.java

+        File f = new File("TaskList.txt");
+        if (!f.exists()) {
+            try {
+                if (f.createNewFile()) {
+                    System.out.println("File created: " + f.getAbsolutePath());
+                } else {
+                    System.out.println("File creation failed.");
+                }
+            } catch (IOException e) {
+                System.out.println("An error occurred.");
+                e.printStackTrace();
+            }
+        } else {
+            System.out.println("File already exists: " + f.getAbsolutePath());
+        }
+    }


Do use more descriptive names for your exceptions and files.
Avoid using magic values (the strings) as well!

nichyjt · 2024-02-23T10:00:57Z

src/main/java/Deadline.java

@@ -0,0 +1,13 @@
+public class Deadline extends Task{


Formatting error here!

nichyjt · 2024-02-23T10:02:49Z

src/main/java/EmptyLineException.java

+public class EmptyLineException extends Exception{
+}


Formatting error for Exception{. Also, I'm not sure why your exception class is empty?

nichyjt · 2024-02-23T10:03:05Z

src/main/java/Duke.java

+    public static void main(String[] args) throws UnexpectedCommandException, EmptyLineException, IOException {
+        ArrayList<Task> tasks = new ArrayList<Task>();
+
+        //greeting


Your comment could be more descriptive!

nichyjt · 2024-02-23T10:03:45Z

src/main/java/Duke.java

+        }
+    }
+
+    public static void main(String[] args) throws UnexpectedCommandException, EmptyLineException, IOException {


Do refactor out your magic string values!

nichyjt · 2024-02-23T10:05:40Z

src/main/java/Event.java

+    }
+    @Override


You can consider adding a newline between line 8 & 9 to avoid making it look too squashed!

nichyjt · 2024-02-23T10:08:08Z

src/main/java/ManageInputs.java

+        while (s.hasNext()) {
+            String sLine = s.nextLine();
+            if (sLine.contains("[E]")) {
+                dealWithEvent(tasks, index, sLine);
+            } else if (sLine.contains("[D]")) {
+                dealWithDeadline(tasks, index, sLine);
+            } else if (sLine.contains("[T]")) {
+                dealWithTodo(tasks, index, sLine);
+            }
+            index++;


This is a good application of SLAP! But perhaps the variable sLine could be renamed to make it clearer what this means?

nichyjt · 2024-02-23T10:12:30Z

src/main/java/ManageInputs.java

+    private void handleUnexpectedCommand(boolean isValidCommand) throws UnexpectedCommandException {
+        if (!isValidCommand) {
+            throw new UnexpectedCommandException();
+        }
+    }


I'm not sure what this abstraction is supposed to do. Is this necessary? If a command is invalid, is it possible to just throw the exception straight?

nichyjt · 2024-02-23T10:13:45Z

src/main/java/ManageInputs.java

+        }
+    }
+
+    public ManageInputs(ArrayList<Task> tasks, int index, String line) throws IOException, UnexpectedCommandException {


This function is doing quite a fair bit of heavy lifting.

It's very long, which usually indicates that perhaps you could apply SLAP more here.

To aid you in achieving SLAP, here are some guiding questions...

What parts of the code is repetitive? i.e. virtually the same for each if/else block you have

What part of the code does a particular function that can be isolated into a function? (like printing tasks, or doing something to the file?)

nichyjt · 2024-02-23T10:20:52Z

src/main/java/ManageInputs.java

+
+            if (inputs[0].equals("mark")) {//mark as done
+                isValidCommand = true;
+                int idx = Integer.parseInt(inputs[1]);


index and idx are being used in the code but seem to be referring to different things. It's not too clear what they really mean! Do consider how you could make this function more readable!

add javadoc

hongyijie06 added 3 commits January 26, 2024 16:59

add chatbot name

f0ecd9c

Add echo

e0b6eed

add Add,list

35ec609

mihirheda02 reviewed Feb 9, 2024

View reviewed changes

hongyijie06 added 2 commits February 9, 2024 15:21

add mark as done

0c16724

add A-CodingStandard

1b6ca15

a-wild-chocolate reviewed Feb 10, 2024

View reviewed changes

hongyijie06 added 12 commits February 16, 2024 22:41

add todo, deadline and event

6ef3078

add code quality

b540106

add error handling

c230255

make error handling more error-specific

bedc8ec

Revert "make error handling more error-specific"

4793c54

This reverts commit bedc8ec.

Revert "Revert "make error handling more error-specific""

c43aef3

This reverts commit 4793c54.

add delete

e0bf4a2

Merge branch 'branch-Level-6'

7078d98

settle errors

4de283c

add save

e44d14e

resolve errors

011c695

Merge branch 'branch-Level-7'

0bc39f6

nichyjt reviewed Feb 23, 2024

View reviewed changes

hongyijie06 and others added 10 commits March 2, 2024 12:04

fix formatting when listing tasks

b1fd82d

add saveToFile

61216db

fix file not found error

996ba2b

use more oop

7162ac7

fix loading file errors

7a5826c

add find

4a60dbe

Merge branch 'branch-Level-9'

422ac12

add javadoc

2152ef7

Merge pull request #1 from hongyijie06/branch-A-JavaDoc

b611a02

add javadoc

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

d141099

hongyijie06 added 6 commits March 8, 2024 02:28

Add user guide

6752306

Format headers

2f4b481

follow coding standards

4f03af2

add README pictures

fbcc37c

add pictures

456bcb0

improve formatting, no pictures

dc9a769

Conversation

hongyijie06 commented Feb 7, 2024

Uh oh!

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

a-wild-chocolate 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!

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

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants