-
Notifications
You must be signed in to change notification settings - Fork 5
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
Create PinCommand feature #99
Create PinCommand feature #99
Conversation
delete command had no functionality to delete via a group. Added this functionality so that it is easier to delete large groups of contacts at once if required.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## add-pin-command #99 +/- ##
=====================================================
- Coverage 76.44% 76.29% -0.16%
- Complexity 521 545 +24
=====================================================
Files 82 84 +2
Lines 1690 1772 +82
Branches 169 187 +18
=====================================================
+ Hits 1292 1352 +60
- Misses 347 360 +13
- Partials 51 60 +9 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall, LGTM!
public class PinCommand extends Command { | ||
public static final String COMMAND_WORD = "pin"; | ||
public static final String MESSAGE_USAGE = COMMAND_WORD + ": Pin a student to the top of the address book. " | ||
+ "Parameters: NusId (8 digits long, starting with an 'E'). \n" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just realised nusid is 7 digits long, not 8
List<Person> lastShownList = model.getFilteredPersonList(); | ||
Person personToPin = lastShownList.stream().filter(person -> person.getNusId().equals(nusId)) | ||
.findFirst().orElse(null); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good use of streams
private static boolean arePrefixesPresent(ArgumentMultimap argumentMultimap, Prefix... prefixes) { | ||
return Stream.of(prefixes).allMatch(prefix -> argumentMultimap.getValue(prefix).isPresent()); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This method could be useful for every future class, consider re-adapting this into ArgumentMultimap class as a public method?
…ete-command Modify delete command
…15-2/delete_feature Able to delete in group for delete feature
* master: Update DeleteCommand.java resolve errors Update DeleteCommandParser.java documentationupdate Update DeleteCommandParser.java Update DeleteCommandParser.java Resolve review update testcases checkstyle Update DeleteCommand.java Update DeleteCommand.java update checkstyle Update DeleteCommandParser.java Update DeleteCommand.java modify-delete-command
Require documentation of pin command: Can be added in another pr |
497e594
into
AY2324S2-CS2103T-T15-2:add-pin-command
for (int i = 0; i < lastShownList.size(); i++) { | ||
if (lastShownList.get(i).getGroups().equals(deletedGroup) ) { | ||
peopleToDelete.add(lastShownList.get(i)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
possible case of deep nesting, perhaps extracting a new method might look better>
Linked to #43, will work on the test cases in the upcoming commit