[Nicholas Nge] iP by nicholasnge · Pull Request #257 · nus-cs2103-AY2021S2/ip

nicholasnge · 2021-02-02T14:55:01Z

No description provided.

# Conflicts: # src/main/java/Deadline.java # src/main/java/Event.java

A-CheckStyle

chanellNg

Overall, the indentation was very consistent and comments were very informative, though some may be a bit lengthy. Naming was consistent and clear.

chanellNg · 2021-02-05T15:55:43Z

src/main/java/command/DeadlineCommand.java

+    }
+
+    /** Creates a new Deadline
+     * @param ui helper to interact with user


Maybe there should be an empty line between the method description and parameter section

chanellNg · 2021-02-05T15:57:23Z

src/main/java/command/EventCommand.java

+
+    /** Creates a new Event
+     * @param ui helper to interact with user
+     * @param tasks we add the new created event here


Perhaps could add punctuation behind each parameter description

good idea, thanks

chanellNg · 2021-02-05T16:04:52Z

src/main/java/oracle/Parser.java

+                ui.showFormatException("FindCommand");
+            }
+            return new EmptyCommand();
+        }


maybe can stick with } else { indentation, comment can be moved up

GJ0407790

Overall, the code is clean and easy to understand.
Just a few things to take note of:

class field should be after the class header.
remember to add a space before "{" in class header.
For the package name, can consider put it all under one big package. eg duke.command, duke.oracle.

GJ0407790 · 2021-02-06T09:00:57Z

src/main/java/command/DeleteCommand.java

+            Task t = tasks.get(this.taskIndex);
+            tasks.remove(this.taskIndex);


Task t = tasks.remove(this.taskIndex)

I think can make this simpler by combining both statements.

good idea, thanks

GJ0407790 · 2021-02-06T09:02:38Z

src/main/java/command/EmptyCommand.java

+import oracle.TaskList;
+import oracle.Ui;
+
+public class EmptyCommand implements Command{


Can have a space after between Command and {

GJ0407790 · 2021-02-06T09:05:08Z

src/main/java/command/DeleteCommand.java

+    /**
+     * @param ui helper to print the new deleted task
+     * @param tasks we call delete on the indicated task in this TaskList
+     * @return true


Can explain why the method always returns true. eg So that duke can continue accepting commands. (I understand this after reading the ExitCommand)

will do, thanks

GJ0407790 · 2021-02-06T09:06:49Z

src/main/java/command/FindCommand.java

+    public boolean execute(Ui ui, TaskList tasks) {
+        ArrayList<Task> results = new ArrayList<>();
+        for (Task t : tasks.getTasks()) {
+            if (t.toString().contains(this.keyword)) {


I think can give a variable to t.toString() to explain whether this is the task description or the string representation of the task.

GJ0407790 · 2021-02-06T09:17:48Z

src/main/java/entry/Deadline.java

+        return res;
+    }
+
+    LocalDateTime deadline;


This field should be right after header?

yup, thanks for pointing it out

GJ0407790 · 2021-02-06T09:20:08Z

src/main/java/entry/Task.java

@@ -0,0 +1,54 @@
+package entry;
+
+public class Task {


Add a header comment to describe the use of this task.
I think can declare Task as an abstract class since there is no Task instance.

GJ0407790 · 2021-02-06T09:21:54Z

src/main/java/oracle/Parser.java

+            return new EmptyCommand();
+        } else if (split[0].equals("find")) {
+            try {
+                String kw = split[1];


Can give a more descriptive name, ie what does kw stands for?

GJ0407790 · 2021-02-06T09:23:04Z

src/main/java/oracle/Parser.java

+     * @param input this is the raw String given by the user
+     * @return a Command
+     */
+    public Command parse(String input) {


Is this method a bit too lengthy, can consider extract out the common part of the code.

GJ0407790 · 2021-02-06T09:23:58Z

src/main/java/oracle/Storage.java

+            FileWriter myWriter = new FileWriter(filePath);
+            for (Task task : tasks){
+                myWriter.write(task.toStorage()+ '\n');
+            }


This indentation is off.

sorry, how is it off? i cant see it

GJ0407790 · 2021-02-06T09:24:44Z

src/main/java/oracle/TaskList.java

+            String[] sorted = s.split("\u001E");
+            Boolean isDone = sorted[1].equals("T");
+            try{
+                switch (sorted[0]) {


Add a default case as specific in textbook.

A-Assertions

…eQuality � Conflicts: � src/main/java/entry/Task.java � src/main/java/oracle/Parser.java � src/main/java/oracle/TaskList.java

A code quality

A-BetterGui

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

Add Gradle support

3b19ba1

build.gradle: Update version to 8.29

a75fcee

Level-1

eb2421a

Level-2

266d212

Level-3

98eb5c0

Level-4

55bc2e6

A-TextUiTesting

abb364e

Update README.md

ce1cba5

Update README.md

cc25792

Update README.md

36ab66b

Update README.md

0ff9ec7

Update README.md

5f13fcc

Level-5

d329a63

Level-6

a4fa223

Merge remote-tracking branch 'origin/master'

7393cc2

nicholasnge changed the title ~~Nicholas Nge iP~~ [Nicholas Nge] iP Feb 2, 2021

nicholasnge added 14 commits February 3, 2021 01:13

Level-7

3542d5f

Level-7

c96e24b

Level-7

0dca6de

Level-8

b9dec69

Merge branch 'branch-Level-8'

b28bd34

# Conflicts: # src/main/java/Deadline.java # src/main/java/Event.java

Level-7 Level-8

cacb22a

A-MoreOOP

c90d25b

A-Packages

f4721d6

Merge remote-tracking branch 'origin/add-gradle-support'

63b29c8

A-JUnit

def2021

A-JavaDoc

628ee47

A-JavaDoc A-Jar

0cfaca4

Level-9

7c2ced8

A-CodingStandard

cb1bd6d

nicholasnge added 3 commits February 4, 2021 04:23

A-Gradle

216cd62

Level-10

4ea02bd

A-CheckStyle

A-CodingStandard

b9b6ab3

chanellNg reviewed Feb 5, 2021

View reviewed changes

GJ0407790 reviewed Feb 6, 2021

View reviewed changes

nicholasnge added 12 commits February 6, 2021 20:58

A-Assertions

2b8d840

A-Assertions

91dbb41

A-CodeQuality

3a88e68

A-CodeQuality

ebfe6a7

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

d7ef1dd

A-Assertions

Merge branch 'master' of https://github.com/nicholasnge/ip into A-Cod…

7d2020c

…eQuality � Conflicts: � src/main/java/entry/Task.java � src/main/java/oracle/Parser.java � src/main/java/oracle/TaskList.java

A-CodeQuality

cb93d3c

A-CodeQuality

db68739

Merge pull request #2 from nicholasnge/A-CodeQuality

317207f

A code quality

B-DoWithinPeriodTasks

d0bbd10

A-UserGuide

61ec8ea

A-BetterGui

Update README.md

63c2f23

		Task t = tasks.get(this.taskIndex);
		tasks.remove(this.taskIndex);

Conversation

nicholasnge commented Feb 2, 2021

Uh oh!

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

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

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants