Create NegateCommand abstract class and add unremind command #118

ChooJiaXin · 2020-10-16T16:42:24Z

closes #109
closes #113

Implemented unremind command as well to ensure the correctness of NegateCommand class

codecov-io · 2020-10-16T20:18:25Z

Codecov Report

Merging #118 into master will increase coverage by 0.58%.
The diff coverage is 85.71%.

@@             Coverage Diff              @@
##             master     #118      +/-   ##
============================================
+ Coverage     55.48%   56.07%   +0.58%     
- Complexity      447      461      +14     
============================================
  Files            90       93       +3     
  Lines          1768     1803      +35     
  Branches        209      213       +4     
============================================
+ Hits            981     1011      +30     
- Misses          739      740       +1     
- Partials         48       52       +4

Impacted Files	Coverage Δ	Complexity Δ
...va/seedu/address/logic/commands/RemindCommand.java	`84.00% <ø> (ø)`	`10.00 <0.00> (ø)`
.../seedu/address/logic/parser/AddressBookParser.java	`80.00% <0.00%> (-4.22%)`	`12.00 <0.00> (ø)`
...eedu/address/logic/parser/RemindCommandParser.java	`100.00% <ø> (ø)`	`2.00 <0.00> (ø)`
.../seedu/address/logic/commands/UnremindCommand.java	`82.60% <82.60%> (ø)`	`10.00 <10.00> (?)`
...va/seedu/address/logic/commands/NegateCommand.java	`100.00% <100.00%> (ø)`	`2.00 <2.00> (?)`
...du/address/logic/parser/UnremindCommandParser.java	`100.00% <100.00%> (ø)`	`2.00 <2.00> (?)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5455e5b...08940bd. Read the comment docs.

itsjerryho

Looks good! Just a few comments haha

itsjerryho · 2020-10-17T07:39:28Z

src/main/java/seedu/address/logic/parser/RemindCommandParser.java

@@ -10,7 +10,7 @@

    /**
     * Parses the given {@code String} of arguments in the context of the DeleteCommand


is this supposed to be 'in the context of RemindCommand' instead of DeleteCommand?

Oh yup. Must have missed it out. I shall go change it now

itsjerryho · 2020-10-17T07:39:57Z

src/main/java/seedu/address/logic/commands/NegateCommand.java

+/**
+ * Represents a command with hidden internal logic and the ability to reverse the execution of other commands.
+ */
+public abstract class NegateCommand extends Command {


does it have to be abstract since this class doesn't contain any abstract methods.. I think a normal class would suffice

At first, I made it a non-abstract class, but then IntelliJ made noise because it extends abstract class Command so i either had to override the execute method in NegataCommand or make NegateCommand abstract. Since I personally felt that overriding the execute method in NegateCommand was a little redundant since the UnremindCommand, UnscheduleCommand and UndoneCommand will override the execute method again, I decided to make the NegateCommand class abstract.

I can change it back to a normal class if you want to though, I shall just override the abstract execute method in NegateCommand if you want to change it to a normal class :)

I think just keep it as an abstract class will do.

oh okay, I didn't know that Command was abstract. ya we can leave it as abstract then

itsjerryho · 2020-10-17T07:40:54Z

src/main/java/seedu/address/logic/parser/UnremindCommandParser.java

+public class UnremindCommandParser implements Parser<UnremindCommand> {
+
+    /**
+     * Parses the given {@code String} of arguments in the context of the DeleteCommand


UnremindCommand?

itsjerryho · 2020-10-17T07:42:59Z

src/test/java/seedu/address/logic/commands/UnremindCommandTest.java

+        model.setAssignment(assignmentInList, assignmentInListReminded);
+    }
+
+    // Reminded first assignment in filtered list


second* assignment

hyngkng

LGTM. :) I think the abstract class issue is minor and we can leave it as it is for now.

hyngkng · 2020-10-17T08:07:22Z

src/main/java/seedu/address/logic/commands/NegateCommand.java

+/**
+ * Represents a command with hidden internal logic and the ability to reverse the execution of other commands.
+ */
+public abstract class NegateCommand extends Command {


I think just keep it as an abstract class will do.

hyngkng · 2020-10-17T08:11:39Z

src/main/java/seedu/address/logic/commands/UnremindCommand.java

+            throw new CommandException(MESSAGE_UNREMINDED_ASSIGNMENT);
+        }
+
+        assert(assignmentToUnremind.isReminded());


Good assertion.

hyngkng · 2020-10-17T08:16:51Z

src/test/java/seedu/address/logic/commands/UnremindCommandTest.java

+import seedu.address.model.assignment.Assignment;
+import seedu.address.testutil.AssignmentBuilder;
+
+public class UnremindCommandTest {


Good test cases. They cover every case I can think of, so good job.

itsjerryho

LGTM! :)

Create NegateCommand abstract class

6d382f4

ChooJiaXin added this to the v1.3 milestone Oct 16, 2020

ChooJiaXin requested review from itsjerryho and hyngkng October 16, 2020 16:42

ChooJiaXin self-assigned this Oct 16, 2020

ChooJiaXin added 5 commits October 17, 2020 00:48

Add JavaDocs

6246436

Implement unremind feature

dd6d87c

Add tests for unremind command and parser

b4a60b4

Merge branch 'master' into branch-negate-command

76fccf3

Fix typos is error message

08940bd

ChooJiaXin changed the title ~~Create NegateCommand abstract class~~ Create NegateCommand abstract class and add unremind command Oct 16, 2020

itsjerryho requested changes Oct 17, 2020

View reviewed changes

Fix typos according to github reviews

e6c06e2

hyngkng approved these changes Oct 17, 2020

View reviewed changes

itsjerryho approved these changes Oct 17, 2020

View reviewed changes

ChooJiaXin merged commit 24463b4 into AY2021S1-CS2103T-F11-3:master Oct 17, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Create NegateCommand abstract class and add unremind command #118

Create NegateCommand abstract class and add unremind command #118

ChooJiaXin commented Oct 16, 2020 •

edited

codecov-io commented Oct 16, 2020

itsjerryho left a comment

itsjerryho Oct 17, 2020

ChooJiaXin Oct 17, 2020

ChooJiaXin Oct 17, 2020

itsjerryho Oct 17, 2020

ChooJiaXin Oct 17, 2020

hyngkng Oct 17, 2020

itsjerryho Oct 17, 2020

itsjerryho Oct 17, 2020

ChooJiaXin Oct 17, 2020

itsjerryho Oct 17, 2020

ChooJiaXin Oct 17, 2020

hyngkng left a comment

hyngkng Oct 17, 2020

hyngkng Oct 17, 2020

hyngkng Oct 17, 2020

itsjerryho left a comment

		@@ -10,7 +10,7 @@

		/**
		* Parses the given {@code String} of arguments in the context of the DeleteCommand

Create NegateCommand abstract class and add unremind command #118

Create NegateCommand abstract class and add unremind command #118

Conversation

ChooJiaXin commented Oct 16, 2020 • edited

codecov-io commented Oct 16, 2020

Codecov Report

itsjerryho 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

hyngkng 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

itsjerryho left a comment

Choose a reason for hiding this comment

ChooJiaXin commented Oct 16, 2020 •

edited