[Chuyue] iP #349

ramenmen · 2020-08-25T04:26:13Z

No description provided.

lamlaaaam

Might want to review the longer methods and try to split up responsibilities more.

lamlaaaam · 2020-08-31T10:23:12Z

src/main/java/Command.java

@@ -0,0 +1,10 @@
+public enum Command {
+    blank,


Perhaps change the constants to all caps? e.g. BLANK, CLEAR, etc.

lamlaaaam · 2020-08-31T10:24:15Z

src/main/java/Deadline.java

+        this.deadline = deadline;
+    }
+    public String toString() {
+        String donez;


Might want to change the variable name to something clearer?

lamlaaaam · 2020-08-31T10:24:36Z

src/main/java/Deadline.java

+    }
+    public String toString() {
+        String donez;
+        if (done) {


"done" seems like a boolean value, maybe try "isDone"?

boolean method can be named isDone() instead.

lamlaaaam · 2020-08-31T10:25:38Z

src/main/java/Duke.java

+    /**
+     * Contains most of the chat bot logic
+     */
+    public void run() {


Perhaps extract out certain parts into helper methods? Currently this method seems too long and will only grow longer as more logic is added.

lamlaaaam · 2020-08-31T10:25:57Z

src/main/java/Duke.java

+                } else {
+                    task = tasks.get(current);
+                    tasks.delete(current);
+                    Ui.print("i've removed the following task from the list:\n\t" + task + "\nnow you have " + tasks.size() + " items in your tasklist.");


Maybe split up this line as it is too long?

lamlaaaam · 2020-08-31T10:27:47Z

src/main/java/Parser.java

+     * @param in The string to be parsed.
+     * @return The task required by the input string.
+     */
+    public static Task getTask(String in) {


Again, maybe consider breaking the method up into several smaller ones?

lamlaaaam · 2020-08-31T10:29:06Z

src/main/java/Storage.java

+        List<Task> result = new ArrayList<Task>();
+        File myObj = new File(filePath);
+        Scanner myReader;
+        try {


This code block is getting close to an arrowhead style! Maybe try to refrain nesting too many try blocks?

lamlaaaam · 2020-08-31T10:30:05Z

src/main/java/Task.java

+        if (taskType.equals("T")) {
+            tempTask = new ToDo(result[2]);
+        } else if (taskType.equals("D")) {
+            tempTask = new Deadline(result[2],LocalDateTime.parse(result[3],DateTimeFormatter.ofPattern("dd MMM yyyy HHmm")));


Parameters look quite long and there's many magic numbers, maybe split up the lines and use constants?

lamlaaaam · 2020-08-31T10:31:11Z

src/main/java/Ui.java

+     * Prints an error message with the associated formatting.
+     * @param msg The message to be printed.
+     */
+    public static void errorMsg(String msg) {


Perhaps change the method name to "printErrorMsg"?

Perhaps change the method name to "printErrorMessage" or "printErrorMsg" as mentioned by Zhi Yuan.

lamlaaaam · 2020-08-31T10:31:51Z

src/test/java/DukeTest.java

+    @Test
+    public void parserTestReturnError(){
+        assertEquals(Parser.parse("dfgjdhg"), Command.error);
+        assertEquals(Parser.parse("todo"), Command.error);


Why does parsing "todo" and "event" expect an error command?

marcustw

Great job! Java OOP adhered!

marcustw · 2020-09-01T07:03:02Z

src/main/java/Command.java

@@ -0,0 +1,10 @@
+public enum Command {
+    blank,
+    clear,


Should it these enum constants be in all caps?

marcustw · 2020-09-01T07:14:51Z

src/main/java/Deadline.java

+    }
+    public String toString() {
+        String donez;
+        if (done) {


boolean method can be named isDone() instead.

marcustw · 2020-09-01T07:16:49Z

src/main/java/Duke.java

-                + "| |_| | |_| |   <  __/\n"
-                + "|____/ \\__,_|_|\\_\\___|\n";
-        System.out.println("Hello from\n" + logo);
+        new Duke("duke.txt").run();


Very clean psvm method!

marcustw · 2020-09-01T07:17:11Z

src/main/java/Event.java

+
+public class Event extends Task {
+    private LocalDateTime startDate;
+    public Event(String in, LocalDateTime start) {


Perhaps a line space between properties and the class constructor.

marcustw · 2020-09-01T07:17:33Z

src/main/java/Deadline.java

+
+public class Deadline extends Task {
+    private LocalDateTime deadline;
+    public Deadline(String in, LocalDateTime deadline) {


Perhaps a line space between properties and class constructor.

marcustw · 2020-09-01T07:24:54Z

src/main/java/Parser.java

+public class Parser {
+    /**
+     * Converts the string to a type of command.
+     * @param in The string to be parsed.


Perhaps add a space between Javadoc description and @param (line 6 and 7).

marcustw · 2020-09-01T07:30:24Z

src/main/java/Task.java

+import java.time.format.DateTimeFormatter;
+
+public class Task {
+    protected boolean done;


Perhaps boolean can be named "isDone"?

marcustw · 2020-09-01T07:31:26Z

src/main/java/TaskList.java

+    }
+
+    /**
+     * Converts tasks in taskList to String format.


Perhaps you can leave a line space between line 43 and line 44

marcustw · 2020-09-01T07:33:17Z

src/main/java/Ui.java

+     * Prints an error message with the associated formatting.
+     * @param msg The message to be printed.
+     */
+    public static void errorMsg(String msg) {


Perhaps change the method name to "printErrorMessage" or "printErrorMsg" as mentioned by Zhi Yuan.

A-Assertions

A-CodeQuality

Add comments and abstract out some methods and constants

damithc and others added 22 commits July 23, 2020 23:27

Add Gradle support

3b19ba1

Level-1

7c1eb15

Level-2

e833d84

Level-3

62e64f8

Level-4

92d6292

Level-6

efa3c3f

Level-5

faad935

Level-6

b56c143

A-TextUiTesting

5b04b51

A-TextUiTesting

b63b58f

Level-6

a225294

Level-7

445dc46

Level-8

4ea6290

Level-8

fd7d34b

A-MoreOOP

b5b5cd6

A-JUnit

2d23096

A-JavaDoc

8231389

A-CodingStandard

5ebaa1f

Level-9

e42e2c8

A-JavaDoc

0067074

A-CodingStandard

32ac8b0

Level-9

0a4a1e6

lamlaaaam reviewed Aug 31, 2020

View reviewed changes

Merge branch 'add-gradle-support' of https://github.com/ramenmen/ip

086ff1b

marcustw reviewed Sep 1, 2020

View reviewed changes

ramenmen added 5 commits September 1, 2020 16:36

A-Gradle

ab03740

Level-10

3c61dad

A-Assertions

5d7578c

A-CodeQuality

1b27357

D-Notes

efef814

ramenmen and others added 13 commits September 8, 2020 23:49

Merge pull request #1 from ramenmen/branch-A-Assertions

3f5e994

A-Assertions

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

fab254d

A-CodeQuality

6795f23

Merge branch 'master' into branch-A-CodeQuality

14f370d

Merge pull request #2 from ramenmen/branch-A-CodeQuality

066b053

A-CodeQuality

resolve conflicts

f52cbdd

Set theme jekyll-theme-cayman

ef2ecfc

A-UserGuide

9700db7

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

e7b57ab

A-UserGuide

937922e

Catch more errors

1f5f97a

Add comments and abstract out some methods and constants

4e584d2

Merge pull request #4 from ramenmen/letmepasspls

7c69d3c

Add comments and abstract out some methods and constants

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Chuyue] iP #349

[Chuyue] iP #349

ramenmen commented Aug 25, 2020

lamlaaaam left a comment

lamlaaaam Aug 31, 2020

lamlaaaam Aug 31, 2020

lamlaaaam Aug 31, 2020

marcustw Sep 1, 2020

lamlaaaam Aug 31, 2020

lamlaaaam Aug 31, 2020

lamlaaaam Aug 31, 2020

lamlaaaam Aug 31, 2020

lamlaaaam Aug 31, 2020

lamlaaaam Aug 31, 2020

marcustw Sep 1, 2020

lamlaaaam Aug 31, 2020

marcustw left a comment

marcustw Sep 1, 2020

marcustw Sep 1, 2020

marcustw Sep 1, 2020

marcustw Sep 1, 2020

marcustw Sep 1, 2020

marcustw Sep 1, 2020

marcustw Sep 1, 2020

marcustw Sep 1, 2020

marcustw Sep 1, 2020

[Chuyue] iP #349

Are you sure you want to change the base?

[Chuyue] iP #349

Conversation

ramenmen commented Aug 25, 2020

lamlaaaam left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

marcustw left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment