Merge pull request #269 from wasjoe1/branch-improve-test-and-code

Branch improve test and code

docs/team/wasjoe1.md

@@ -23,7 +23,8 @@ Given below are my contributions to the project.
 * **New Feature**: Added the ability to filter guests and vendors.
     * What it does: allows the user to filter guest and vendor lists by all non-tag parameters.
     * Justification: This feature improves the product significantly because a user can now filter the guest and vendor list by more parameters, giving them a more customised filter feature.
-    * Highlights: The implementation was challenging because each parameter had a respective predicate class which accepted different types of inputs respectively. This required careful consideration of possible user inputs.
+    * Highlights: The implementation was challenging because each parameter had a respective predicate class which accepted different types of inputs respectively. This required careful consideration of possible user inputs.* 
+      Also employed defensive programming practices in implementation and applied test heuristics in test case design (observed in Predicate classes e.g. `NamePredicate` and `NamePredicateTest`).
     * Credits: Partially adapted from `FindCommand` and `NameContainsKeywordsPredicate` class in [AY2324S1-CS2103T-W08-3](https://github.com/AY2324S1-CS2103T-W08-3/tp/).
 
 * **New Feature**: Modified the guest class to take in Dietary requirement field (initial implementation).
@@ -40,8 +41,10 @@ Given below are my contributions to the project.
     * User Guide:
         * Added documentation for the features `guest list` and `vendor list`: [\#40](https://github.com/AY2324S1-CS2103T-F11-2/tp/pull/40/)
         * Added documentation for the features `guest filter` and `vendor filter`: [\#121](https://github.com/AY2324S1-CS2103T-F11-2/tp/pull/121)
+        * Added documentation for the feature `clear`: [\#121](https://github.com/AY2324S1-CS2103T-F11-2/tp/pull/121)
         * Added documentation for dietary requirements field: [\#98](https://github.com/AY2324S1-CS2103T-F11-2/tp/pull/98)
         * Vetted UG before PED: [\#174](https://github.com/AY2324S1-CS2103T-F11-2/tp/pull/174), [\#175](https://github.com/AY2324S1-CS2103T-F11-2/tp/pull/175)
+        * Updated Appendix: [\#267](https://github.com/AY2324S1-CS2103T-F11-2/tp/pull/267)
     * Developer Guide:
         * Added use case section: [\#38](https://github.com/AY2324S1-CS2103T-F11-2/tp/pull/38)
         * Added implementation details for the feature `filter`: [\#145](https://github.com/AY2324S1-CS2103T-F11-2/tp/pull/145)

src/main/java/wedlog/address/logic/commands/GuestFilterCommand.java

@@ -49,7 +49,12 @@ public class GuestFilterCommand extends Command {
 
     private final List<Predicate<? super Guest>> predicates;
 
+    /**
+     * Creates a GuestFilterCommand which filters according to {@code predicates}
+     */
     public GuestFilterCommand(List<Predicate<? super Guest>> predicates) {
+        // Defensive programming by not allowing a null to be assigned to predicates
+        assert predicates != null : "Predicates passed to GuestFilterCommand should not be null!";
         this.predicates = predicates;
     }
 

src/main/java/wedlog/address/logic/commands/VendorFilterCommand.java

@@ -40,7 +40,12 @@ public class VendorFilterCommand extends Command {
 
     private final List<Predicate<? super Vendor>> predicates;
 
+    /**
+     * Creates a VendorFilterCommand which filters according to {@code predicates}
+     */
     public VendorFilterCommand(List<Predicate<? super Vendor>> predicates) {
+        // Defensive programming by not allowing a null to be assigned to predicates
+        assert predicates != null : "Predicates passed to VendorFilterCommand should not be null!";
         this.predicates = predicates;
     }
 

src/main/java/wedlog/address/logic/parser/GuestCommandParser.java

@@ -48,7 +48,7 @@ public Command parseCommand(String userInput) throws ParseException {
         // Note to developers: Change the log level in config.json to enable lower level (i.e., FINE, FINER and lower)
         // log messages such as the one below.
         // Lower level log messages are used sparingly to minimize noise in the code.
-        logger.fine("Command word: " + commandWord + "; Arguments: " + arguments);
+        logger.fine("Guest command word: " + commandWord + "; Arguments: " + arguments);
 
         switch (commandWord) {
         case GuestAddCommand.COMMAND_WORD:
@@ -67,7 +67,7 @@ public Command parseCommand(String userInput) throws ParseException {
             return new GuestFilterCommandParser().parse(arguments);
 
         default:
-            logger.finer("This user input caused a ParseException: " + userInput);
+            logger.finer("This user input for guest command caused a ParseException: " + userInput);
             throw new ParseException(MESSAGE_UNKNOWN_COMMAND);
         }
     }

src/main/java/wedlog/address/logic/parser/VendorCommandParser.java

@@ -49,7 +49,7 @@ public Command parseCommand(String userInput) throws ParseException {
         // Note to developers: Change the log level in config.json to enable lower level (i.e., FINE, FINER and lower)
         // log messages such as the one below.
         // Lower level log messages are used sparingly to minimize noise in the code.
-        logger.fine("Command word: " + commandWord + "; Arguments: " + arguments);
+        logger.fine("Vendor command word: " + commandWord + "; Arguments: " + arguments);
 
         switch (commandWord) {
         case VendorAddCommand.COMMAND_WORD:
@@ -67,7 +67,7 @@ public Command parseCommand(String userInput) throws ParseException {
         case VendorFilterCommand.COMMAND_WORD:
             return new VendorFilterCommandParser().parse(arguments);
         default:
-            logger.finer("This user input caused a ParseException: " + userInput);
+            logger.finer("This user input for vendor command caused a ParseException: " + userInput);
             throw new ParseException(MESSAGE_UNKNOWN_COMMAND);
         }
     }

src/main/java/wedlog/address/model/person/AddressPredicate.java

@@ -19,6 +19,7 @@ public AddressPredicate(String input) {
 
     @Override
     public boolean test(Person person) {
+        assert person != null : "Person passed to AddressPredicate should not be null!";
         return input.isEmpty()
                 ? person.getAddress().isEmpty() // if input is "", return if field is empty
                 : person.getAddress() // else check if input is contained in the field value

src/main/java/wedlog/address/model/person/EmailPredicate.java

@@ -19,6 +19,7 @@ public EmailPredicate(String input) {
 
     @Override
     public boolean test(Person person) {
+        assert person != null : "Person passed to EmailPredicate should not be null!";
         return input.isEmpty()
                 ? person.getEmail().isEmpty() // if input is "", return if field is empty
                 : person.getEmail() // else check if input is contained in the field value

src/main/java/wedlog/address/model/person/GuestRsvpPredicate.java

@@ -19,6 +19,7 @@ public GuestRsvpPredicate(String input) {
 
     @Override
     public boolean test(Guest guest) {
+        assert guest != null : "Guest passed to GuestRsvpPredicate should not be null!";
         return input.isEmpty()
                 ? guest.getRsvpStatus().value.toLowerCase().contains("unknown") // check if value is unknown
                 : guest.getRsvpStatus().value.equalsIgnoreCase(input); // check if value is exactly same

src/main/java/wedlog/address/model/person/GuestTablePredicate.java

@@ -19,6 +19,7 @@ public GuestTablePredicate(String input) {
 
     @Override
     public boolean test(Guest guest) {
+        assert guest != null : "Guest passed to GuestTablePredicate should not be null!";
         return input.isEmpty()
                 ? guest.getTableNumber().isEmpty() // if input is "", return if field is empty
                 : guest.getTableNumber() // else check if input is contained in the field value

src/main/java/wedlog/address/model/person/NamePredicate.java

@@ -19,6 +19,7 @@ public NamePredicate(String input) {
 
     @Override
     public boolean test(Person person) {
+        assert person != null : "Person passed to NamePredicate should not be null!";
         return input.isEmpty()
                 ? false // if input is "", return false
                 : person.getName().fullName.toLowerCase().contains(input.toLowerCase());

src/main/java/wedlog/address/model/person/PhonePredicate.java

@@ -19,6 +19,7 @@ public PhonePredicate(String input) {
 
     @Override
     public boolean test(Person person) {
+        assert person != null : "Person passed to PhonePredicate should not be null!";
         return input.isEmpty()
                 ? person.getPhone().isEmpty() // if input is "", return if field is empty
                 : person.getPhone() // else check if input is contained in the field value

src/main/java/wedlog/address/model/tag/GuestDietaryPredicate.java

@@ -28,6 +28,7 @@ public GuestDietaryPredicate(List<String> keywords) {
      */
     @Override
     public boolean test(Guest guest) {
+        assert guest != null : "Guest passed to GuestDietaryPredicate should not be null!";
         assert !keywords.isEmpty() : "keywords list for GuestDietaryPredicate should not be empty";
         return keywords.get(0).isEmpty() // If a keyword is empty
                 ? guest.getDietaryRequirements().isEmpty() // Return true if DR field is also empty

src/main/java/wedlog/address/model/tag/TagPredicate.java

@@ -27,6 +27,7 @@ public TagPredicate(List<String> keywords) {
      */
     @Override
     public boolean test(Person person) {
+        assert person != null : "Person passed to TagPredicate should not be null!";
         assert !keywords.isEmpty() : "keywords list for TagPredicate should not be empty";
         return keywords.get(0).isEmpty() // If a keyword is empty
                 ? person.getTags().isEmpty() // Return true if Tag field is also empty

src/test/java/wedlog/address/logic/commands/GuestFilterCommandTest.java

@@ -19,6 +19,7 @@
 import wedlog.address.model.UserPrefs;
 import wedlog.address.model.person.Guest;
 import wedlog.address.model.person.NamePredicate;
+import wedlog.address.testutil.Assert;
 
 class GuestFilterCommandTest {
     private Model model = new ModelManager(getTypicalAddressBook(), new UserPrefs());
@@ -53,6 +54,22 @@ public void equals() {
         assertFalse(filterFirstCommand.equals(filterSecondCommand));
     }
 
+    @Test
+    public void testAssertionPersonNonNull() {
+        NamePredicate predicate = prepareNamePredicate("Alice");
+        List<Predicate<? super Guest>> predicates = Collections.singletonList(predicate);
+
+        // Non null scenario
+        assertTrue(new GuestFilterCommand(predicates) instanceof GuestFilterCommand);
+
+        // Heuristic: No more than 1 invalid input in a test case
+        // Null scenario
+        List<Predicate<? super Guest>> nullPredicates = null;
+        String expectedErrMsg = "Predicates passed to GuestFilterCommand should not be null!";
+        Assert.assertThrows(AssertionError.class,
+                expectedErrMsg, () -> new GuestFilterCommand(nullPredicates));
+    }
+
     @Test
     public void execute_noKeywords_noGuestFound() {
         String expectedMessage = String.format(MESSAGE_GUESTS_LISTED_OVERVIEW, 0);

src/test/java/wedlog/address/logic/commands/VendorFilterCommandTest.java

@@ -19,6 +19,7 @@
 import wedlog.address.model.UserPrefs;
 import wedlog.address.model.person.NamePredicate;
 import wedlog.address.model.person.Vendor;
+import wedlog.address.testutil.Assert;
 
 class VendorFilterCommandTest {
     private Model model = new ModelManager(getTypicalAddressBook(), new UserPrefs());
@@ -53,6 +54,22 @@ public void equals() {
         assertFalse(filterFirstCommand.equals(filterSecondCommand));
     }
 
+    @Test
+    public void testAssertionPersonNonNull() {
+        NamePredicate predicate = prepareNamePredicate("Alice");
+        List<Predicate<? super Vendor>> predicates = Collections.singletonList(predicate);
+
+        // Non null scenario
+        assertTrue(new VendorFilterCommand(predicates) instanceof VendorFilterCommand);
+
+        // Heuristic: No more than 1 invalid input in a test case
+        // Null scenario
+        List<Predicate<? super Vendor>> nullPredicates = null;
+        String expectedErrMsg = "Predicates passed to VendorFilterCommand should not be null!";
+        Assert.assertThrows(AssertionError.class,
+                expectedErrMsg, () -> new VendorFilterCommand(nullPredicates));
+    }
+
     @Test
     public void execute_noKeywords_noVendorFound() {
         String expectedMessage = String.format(MESSAGE_VENDORS_LISTED_OVERVIEW, 0);
@@ -72,6 +89,7 @@ public void execute_singleKeyword_singleVendorFound() {
         assertCommandSuccess(command, model, expectedMessage, expectedModel);
         assertEquals(Collections.singletonList(ANNE), model.getFilteredVendorList());
     }
+
     @Test
     public void toStringMethod() {
         List<Predicate<? super Vendor>> predicates = Collections.singletonList(new NamePredicate("keyword1"));

src/test/java/wedlog/address/model/person/AddressPredicateTest.java

@@ -1,12 +1,12 @@
 package wedlog.address.model.person;
 
-
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertFalse;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 
 import org.junit.jupiter.api.Test;
 
+import wedlog.address.testutil.Assert;
 import wedlog.address.testutil.PersonBuilder;
 
 class AddressPredicateTest {
@@ -36,6 +36,21 @@ public void equals() {
         assertFalse(firstPredicate.equals(null));
     }
 
+    @Test
+    public void testAssertionPersonNonNull() {
+        AddressPredicate pred = new AddressPredicate("Jurong");
+
+        // Non null scenario
+        Person person = new PersonBuilder().withAddress("Jurong").build();
+        assertTrue(pred.test(person));
+
+        // Heuristic: No more than 1 invalid input in a test case
+        // Null scenario
+        Person nullPerson = null;
+        Assert.assertThrows(AssertionError.class,
+                "Person passed to AddressPredicate should not be null!", () -> pred.test(nullPerson));
+    }
+
     @Test
     public void test_addressContainsInput_returnsTrue() {
         // Exact match

src/test/java/wedlog/address/model/person/EmailPredicateTest.java

@@ -6,6 +6,7 @@
 
 import org.junit.jupiter.api.Test;
 
+import wedlog.address.testutil.Assert;
 import wedlog.address.testutil.PersonBuilder;
 
 class EmailPredicateTest {
@@ -35,6 +36,21 @@ public void equals() {
         assertFalse(firstPredicate.equals(null));
     }
 
+    @Test
+    public void testAssertionPersonNonNull() {
+        EmailPredicate pred = new EmailPredicate("[email protected]");
+
+        // Non null scenario
+        Person person = new PersonBuilder().withEmail("[email protected]").build();
+        assertTrue(pred.test(person));
+
+        // Heuristic: No more than 1 invalid input in a test case
+        // Null scenario
+        Person nullPerson = null;
+        Assert.assertThrows(AssertionError.class,
+                "Person passed to EmailPredicate should not be null!", () -> pred.test(nullPerson));
+    }
+
     @Test
     public void test_emailContainsInput_returnsTrue() {
         // Exact match

src/test/java/wedlog/address/model/person/GuestRsvpPredicateTest.java

@@ -6,6 +6,7 @@
 
 import org.junit.jupiter.api.Test;
 
+import wedlog.address.testutil.Assert;
 import wedlog.address.testutil.GuestBuilder;
 
 class GuestRsvpPredicateTest {
@@ -35,6 +36,21 @@ public void equals() {
         assertFalse(firstPredicate.equals(null));
     }
 
+    @Test
+    public void testAssertionGuestNonNull() {
+        GuestRsvpPredicate pred = new GuestRsvpPredicate("yes");
+
+        // Non null scenario
+        Guest guest = new GuestBuilder().withRsvpStatus("yes").build();
+        assertTrue(pred.test(guest));
+
+        // Heuristic: No more than 1 invalid input in a test case
+        // Null scenario
+        Guest nullGuest = null;
+        Assert.assertThrows(AssertionError.class,
+                "Guest passed to GuestRsvpPredicate should not be null!", () -> pred.test(nullGuest));
+    }
+
     @Test
     public void test_rsvpContainsInput_returnsTrue() {
         // Exact match

src/test/java/wedlog/address/model/person/GuestTablePredicateTest.java

@@ -6,6 +6,7 @@
 
 import org.junit.jupiter.api.Test;
 
+import wedlog.address.testutil.Assert;
 import wedlog.address.testutil.GuestBuilder;
 
 class GuestTablePredicateTest {
@@ -35,6 +36,21 @@ public void equals() {
         assertFalse(firstPredicate.equals(null));
     }
 
+    @Test
+    public void testAssertionGuestNonNull() {
+        GuestTablePredicate pred = new GuestTablePredicate("111");
+
+        // Non null scenario
+        Guest guest = new GuestBuilder().withTableNumber("111").build();
+        assertTrue(pred.test(guest));
+
+        // Heuristic: No more than 1 invalid input in a test case
+        // Null scenario
+        Guest nullGuest = null;
+        Assert.assertThrows(AssertionError.class,
+                "Guest passed to GuestTablePredicate should not be null!", () -> pred.test(nullGuest));
+    }
+
     @Test
     public void test_tableNumberContainsInput_returnsTrue() {
         // Exact match

src/test/java/wedlog/address/model/person/NamePredicateTest.java

@@ -6,6 +6,7 @@
 
 import org.junit.jupiter.api.Test;
 
+import wedlog.address.testutil.Assert;
 import wedlog.address.testutil.PersonBuilder;
 
 class NamePredicateTest {
@@ -35,28 +36,43 @@ public void equals() {
         assertFalse(firstPredicate.equals(secondPredicate));
     }
 
+    @Test
+    public void testAssertionPersonNonNull() {
+        NamePredicate pred = new NamePredicate("Alice");
+
+        // EP1: Non null scenario
+        Person person = new PersonBuilder().withName("Alice").build();
+        assertTrue(pred.test(person));
+
+        // Heuristic: No more than 1 invalid input in a test case
+        // EP2: Null scenario
+        Person nullPerson = null;
+        Assert.assertThrows(AssertionError.class,
+                "Person passed to NamePredicate should not be null!", () -> pred.test(nullPerson));
+    }
+
     @Test
     public void test_nameContainsInput_returnsTrue() {
-        // Exact match
+        // EP1: Exact match
         NamePredicate predicate = new NamePredicate("Alice");
         assertTrue(predicate.test(new PersonBuilder().withName("Alice").build()));
 
-        // Partial match
+        // EP2: Partial match
         predicate = new NamePredicate("Alice C");
         assertTrue(predicate.test(new PersonBuilder().withName("Alice Carol").build()));
 
-        // Mixed-case input
+        // EP3: Mixed-case input
         predicate = new NamePredicate("aLIce bOB");
         assertTrue(predicate.test(new PersonBuilder().withName("Alice Bob").build()));
     }
 
     @Test
     public void test_nameDoesNotContainKeywords_returnsFalse() {
-        // Empty input
+        // EP4: Empty input
         NamePredicate predicate = new NamePredicate("");
         assertFalse(predicate.test(new PersonBuilder().withName("Alice").build()));
 
-        // Non-matching input
+        // EP5: Non-matching input
         predicate = new NamePredicate("Carol");
         assertFalse(predicate.test(new PersonBuilder().withName("Alice Bob").build()));
     }