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

Finish up model package #75

Merged

Conversation

lyeyixian
Copy link
Collaborator

Don't merge first, I haven't finish yet

@lyeyixian lyeyixian linked an issue Oct 5, 2020 that may be closed by this pull request
@lyeyixian lyeyixian added this to the v1.2 milestone Oct 5, 2020
Fix some typos, javadocs and import statement issues
Update NameContainsKeywordsPredicate to work properly
@lyeyixian
Copy link
Collaborator Author

I think I'm done for now? Hopes everything works.

@lyeyixian lyeyixian added this to In progress in v1.2 via automation Oct 6, 2020
Copy link
Collaborator

@jeannetoh99 jeannetoh99 left a comment

Choose a reason for hiding this comment

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

Overall looks good, just that I think Name and NameContainsKeywordsPredicate classes should also be extended for its use by TravelPlan.

src/main/java/seedu/address/model/commons/Name.java Outdated Show resolved Hide resolved
*/
public class NameContainsKeywordsPredicate implements Predicate<Activity> {
public class NameContainsKeywordsPredicate implements Predicate<TravelPlanObject> {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe here can also implement Predicate, then we can overload the test method with test(TravelPlan tp) as well?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I updated it. But I don't know if it works, can help me check?

Copy link
Collaborator

@jeannetoh99 jeannetoh99 left a comment

Choose a reason for hiding this comment

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

Hi! Added some comments. Let me know if you have further clarifications. Thanks!!

*/
public class NameContainsKeywordsPredicate implements Predicate<Activity> {
public class NameContainsKeywordsPredicate implements Predicate<Object> {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I meant implements Predicate<TravelPlanObject>, Predicate<TravelPlan> since a class can implement more than one interfaces.

Comment on lines +20 to +30
public boolean test(Object obj) {
if (obj instanceof TravelPlan) {
return keywords.stream()
.anyMatch(keyword -> StringUtil.containsWordIgnoreCase(((TravelPlan) obj).getName().name, keyword));
} else if (obj instanceof TravelPlanObject) {
return keywords.stream()
.anyMatch(keyword ->
StringUtil.containsWordIgnoreCase(((TravelPlanObject) obj).getName().name, keyword));
} else {
return false;
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

As for this, 2 separate (overloaded) test methods:
public boolean test(TravelPlan travelPlan) and public boolean test(TravelPlanObject tpObject)

@lyeyixian lyeyixian merged commit b909d50 into AY2021S1-CS2103-T14-3:master Oct 6, 2020
v1.2 automation moved this from In progress to Done Oct 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
v1.2
Done
Development

Successfully merging this pull request may close these issues.

Update model package
2 participants