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

Persistent search #130

Merged
merged 30 commits into from
Nov 4, 2024
Merged

Persistent search #130

merged 30 commits into from
Nov 4, 2024

Conversation

jan-kai1
Copy link
Collaborator

@jan-kai1 jan-kai1 commented Oct 29, 2024

Trial Implementation of a persistent search function to group multiple selections

Issues

UI
Have NOT implemented UI for the full list of Persons
Currently no way to see who you are trying to add

## Testing
Unsure how to check equality for SearchModeSearchCommands due to difficulty in checking Predicate Equality
Commands generated by SearchModeSearchCommandParser are based on Strings, making it difficult to check equality of different SearchModeSearchCommands generated.
No idea how to check equality of different Predicates

Testing of equality of functions used for FieldContainsKeywordPredicate
FieldContainsKeyWordPredicate currently uses a lambda function to parse out the target field from a person object. May be difficult to test equality
Alternative: Separate each field Predicate into its own separate method.

searchmode implementation

searchmode is tracked by a boolean value searchMode in the ModelManager class. When searchMode is true, the commands accepted by the app changes.

ModelManager also tracks the last used Predicate used to filter its list lastPredicate
The displayed list will be modified by chaining lastPredicate with Predicates generated based on user parameter input using FieldContainsKeywordPredicate class

Enter search mode using searchmode

Searchmode commands

exitsearchmode
Exits searchmode, returning to view of all contacts

search n/{String} e/{String p/ ...
Searches for contacts that matches ALL search parameter criteria

The search parsing currently implemented to check if a person field contains or contains a word equal to the string
This is done in the using the FieldContainsKeywordsPredicate class

This is currently implemented using a SearchModeSearchCommand class that tracks a Set
Upon .execute, the Set is reduced using the .and method to create a final predicate used to filter the list

The predicates are currently stored in a set rather than immediately chaining them using .and due to equality checking

Chaining Searches

Searches with multiple criteria can be chained using separate search commands
search n/Alex
would first add to the displayed list all Persons with name containing Alex

search n/Roy
Adds again to the current displayed list all Persons with name containing Roy

This is done using .or method to combine the new SearchModeSearchCommand predicate with lastPredicate in model manager

Copy link

codecov bot commented Oct 29, 2024

Codecov Report

Attention: Patch coverage is 80.71066% with 38 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
...ss/logic/parser/SearchModeSearchCommandParser.java 86.20% 7 Missing and 1 partial ⚠️
...gic/commands/searchmode/InitSearchModeCommand.java 0.00% 6 Missing ⚠️
...redicates/PhoneNumberContainsKeywordPredicate.java 72.22% 2 Missing and 3 partials ⚠️
...son/predicates/EmailContainsKeywordsPredicate.java 76.47% 1 Missing and 3 partials ⚠️
.../predicates/TelegramContainsKeywordsPredicate.java 77.77% 2 Missing and 2 partials ⚠️
...rc/main/java/seedu/address/logic/LogicManager.java 25.00% 2 Missing and 1 partial ⚠️
.../seedu/address/logic/parser/AddressBookParser.java 76.92% 2 Missing and 1 partial ⚠️
...c/commands/searchmode/SearchModeSearchCommand.java 86.66% 1 Missing and 1 partial ⚠️
...rson/predicates/NameContainsKeywordsPredicate.java 86.66% 0 Missing and 2 partials ⚠️
...n/predicates/AddressContainsKeywordsPredicate.java 94.44% 0 Missing and 1 partial ⚠️
Files with missing lines Coverage Δ Complexity Δ
...s/logic/commands/contact/commands/FindCommand.java 100.00% <ø> (ø) 6.00 <0.00> (ø)
...logic/commands/contact/commands/SearchCommand.java 100.00% <ø> (ø) 6.00 <0.00> (ø)
...gic/commands/searchmode/ExitSearchModeCommand.java 100.00% <100.00%> (ø) 5.00 <5.00> (?)
...logic/parser/contact/parser/FindCommandParser.java 100.00% <ø> (ø) 3.00 <0.00> (ø)
...gic/parser/contact/parser/SearchCommandParser.java 100.00% <ø> (ø) 4.00 <0.00> (ø)
src/main/java/seedu/address/model/Model.java 100.00% <ø> (ø) 1.00 <0.00> (ø)
...rc/main/java/seedu/address/model/ModelManager.java 100.00% <100.00%> (ø) 31.00 <4.00> (+3.00)
...model/person/predicates/PersonIsRolePredicate.java 100.00% <100.00%> (ø) 8.00 <1.00> (?)
src/main/java/seedu/address/model/role/Role.java 100.00% <ø> (ø) 14.00 <0.00> (ø)
...n/predicates/AddressContainsKeywordsPredicate.java 94.44% <94.44%> (ø) 12.00 <12.00> (?)
... and 9 more

@jan-kai1 jan-kai1 self-assigned this Nov 1, 2024
@jan-kai1 jan-kai1 added this to the v1.5 milestone Nov 1, 2024
@jan-kai1 jan-kai1 added type.Enhancement An enhancement to an existing story priority.Medium Nice to have labels Nov 1, 2024
@jan-kai1 jan-kai1 marked this pull request as ready for review November 1, 2024 07:05
Comment on lines 51 to 94
if (argMultimap.getValue(PREFIX_NAME).isPresent()) {
String name = argMultimap.getValue(PREFIX_NAME).get().trim();

String[] nameKeywords = name.split("\\s+");

Predicate<Person> namePred = new NameContainsKeywordsPredicate(
Arrays.stream(nameKeywords).toList());
predicates.add(namePred);
}
if (argMultimap.getValue(PREFIX_PHONE).isPresent()) {
String phone = argMultimap.getValue(PREFIX_PHONE).get().trim();
String[] phoneKeywords = phone.split("\\s+");

Predicate<Person> phonePred = new PhoneNumberContainsKeywordPredicate(
Arrays.stream(phoneKeywords).toList());
predicates.add(phonePred);
}
if (argMultimap.getValue(PREFIX_EMAIL).isPresent()) {
String email = argMultimap.getValue(PREFIX_EMAIL).get().trim();
String[] emailKeywords = email.split("\\s+");

Predicate<Person> emailPred = new EmailContainsKeywordsPredicate(
Arrays.stream(emailKeywords).toList());
predicates.add(emailPred);
}
if (argMultimap.getValue(PREFIX_ADDRESS).isPresent()) {
String address = argMultimap.getValue(PREFIX_ADDRESS).get().trim();
String[] addressKeywords = address.split("\\s+");
Predicate<Person> addressPred = new AddressContainsKeywordsPredicate(
Arrays.stream(addressKeywords).toList());

predicates.add(addressPred);
}
if (argMultimap.getValue(PREFIX_TELEGRAM).isPresent()) {
String telegram = argMultimap.getValue(PREFIX_TELEGRAM).get().trim();
String[] telegramKeywords = telegram.split("\\s+");
Predicate<Person> telegramPred = new TelegramContainsKeywordsPredicate(
Arrays.stream(telegramKeywords).toList());
predicates.add(telegramPred);
}
//role have to use separate predicate
if (argMultimap.getValue(PREFIX_ROLE).isPresent()) {
String roles = argMultimap.getValue(PREFIX_ROLE).get().trim();
// map each word in String roles to a Role object
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very long method, perhaps can refactor the code by abstracting each if block into a separate method for better readability

Copy link

@cshao02 cshao02 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Just some areas that may need some refactoring

@cshao02 cshao02 merged commit a6df416 into master Nov 4, 2024
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority.Medium Nice to have type.Enhancement An enhancement to an existing story
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants