Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Branch improve test and code #269

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion docs/team/wasjoe1.md
Original file line number Diff line number Diff line change
Expand Up @@ -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).
Expand All @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions src/main/java/wedlog/address/model/tag/TagPredicate.java
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down Expand Up @@ -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);
Expand All @@ -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"));
Expand Down
Original file line number Diff line number Diff line change
@@ -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 {
Expand Down Expand Up @@ -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
Expand Down
16 changes: 16 additions & 0 deletions src/test/java/wedlog/address/model/person/EmailPredicateTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

import org.junit.jupiter.api.Test;

import wedlog.address.testutil.Assert;
import wedlog.address.testutil.PersonBuilder;

class EmailPredicateTest {
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

import org.junit.jupiter.api.Test;

import wedlog.address.testutil.Assert;
import wedlog.address.testutil.GuestBuilder;

class GuestRsvpPredicateTest {
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

import org.junit.jupiter.api.Test;

import wedlog.address.testutil.Assert;
import wedlog.address.testutil.GuestBuilder;

class GuestTablePredicateTest {
Expand Down Expand Up @@ -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
Expand Down
26 changes: 21 additions & 5 deletions src/test/java/wedlog/address/model/person/NamePredicateTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

import org.junit.jupiter.api.Test;

import wedlog.address.testutil.Assert;
import wedlog.address.testutil.PersonBuilder;

class NamePredicateTest {
Expand Down Expand Up @@ -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()));
}
Expand Down
Loading
Loading