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.

Sign up for GitHub

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

Jump to bottom

Reformat find by name command #51

Merged
merged 6 commits into from
Oct 9, 2024
Merged

Reformat find by name command #51

merged 6 commits into from
Oct 9, 2024

Conversation

KrashKart
Copy link

@KrashKart KrashKart commented Oct 9, 2024 •
edited
Loading

Intermediate step for find sub-category implementation (find by name, contact, email)

  • Create a temporary AbstractFindCommand class (to replace FindCommand later)
  • Change ContainsKeywordPredicate hierachy (separate into Name-, Contact0 and Email-, all extending from ContainsKeywordPredicate abstract class)
  • Other code format changes

Fixes #50

@KrashKart
Add hierachy to ContainsKeywordsPredicate
adca7b5 
Let's
* create an abstract ContainsKeywordsPredicate class
* let Name-, Contact- and EmailContainsKeywordsPredicate inherit
from the abstract class

This is done to create a hierachy of Predicates used for the
various find commands: find by name, contact and email
@KrashKart
Make FindCommand abstract
c483bc1 
This is so that we can use this as a template for the sub-commands
for Find
@KrashKart
Change message usage and other methods
a44950e 
Temporarily correct until we implement the sub-categories of find
@KrashKart
Add AbstractFindCommand
5ffbc0e 
Temporary so the code does not break
@KrashKart KrashKart added type.Chore Something that needs to be done, but not a story, bug, or an epic. priority.High Must do labels Oct 9, 2024
@KrashKart KrashKart added this to the v1.2 milestone Oct 9, 2024
@KrashKart KrashKart self-assigned this Oct 9, 2024
@KrashKart KrashKart requested a review from yooplo October 9, 2024 16:08
@KrashKart KrashKart marked this pull request as draft October 9, 2024 16:09
@KrashKart KrashKart marked this pull request as ready for review October 9, 2024 16:14
@codecov Codecov
Copy link

codecov bot commented Oct 9, 2024 •
edited
Loading

Codecov Report

Attention: Patch coverage is 19.64286% with 45 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
...du/address/logic/commands/AbstractFindCommand.java 0.00% 16 Missing ⚠️
...model/person/ContactContainsKeywordsPredicate.java 0.00% 11 Missing ⚠️
...s/model/person/EmailContainsKeywordsPredicate.java 0.00% 11 Missing ⚠️
...ddress/model/person/ContainsKeywordsPredicate.java 36.36% 7 Missing ⚠️
Files with missing lines Coverage Δ Complexity Δ
...java/seedu/address/logic/commands/FindCommand.java 100.00% <100.00%> (ø) 6.00 <0.00> (ø)
...ss/model/person/NameContainsKeywordsPredicate.java 100.00% <100.00%> (ø) 7.00 <3.00> (ø)
...ddress/model/person/ContainsKeywordsPredicate.java 36.36% <36.36%> (ø) 2.00 <2.00> (?)
...model/person/ContactContainsKeywordsPredicate.java 0.00% <0.00%> (ø) 0.00 <0.00> (?)
...s/model/person/EmailContainsKeywordsPredicate.java 0.00% <0.00%> (ø) 0.00 <0.00> (?)
...du/address/logic/commands/AbstractFindCommand.java 0.00% <0.00%> (ø) 0.00 <0.00> (?)

@KrashKart
Swap both classes
d5b0b4c 
I super sot
yooplo
yooplo approved these changes Oct 9, 2024
View reviewed changes
Copy link

@yooplo yooplo left a comment

Choose a reason for hiding this comment

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

looks good to me

@yooplo yooplo merged commit c9a4c6c into AY2425S1-CS2103T-F14a-4:master Oct 9, 2024
3 of 4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yooplo yooplo yooplo approved these changes

Assignees

@KrashKart KrashKart

Labels
priority.High Must do type.Chore Something that needs to be done, but not a story, bug, or an epic.
Projects
None yet
Milestone
v1.2
Development

Successfully merging this pull request may close these issues.

Reformat find by name command
2 participants
@KrashKart @yooplo