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

implement ExpressionBuilder->memberOf() method #66

Merged
merged 5 commits into from
Mar 27, 2016

Conversation

ferjul17
Copy link
Contributor

@ferjul17 ferjul17 commented Sep 4, 2015

Hi,

The comparison MEMBER_OF was not implemented on Cretieria while it seems really easy to implement.
I implemented it and wrote the test.
I also tested it in my application.

@baileylo
Copy link
Contributor

The parameter order seems a little odd to me. I'm assuming, the value parameter comes first to make the code more readable, ->memberOf('photography', 'hobbies') could be read 'photography is a member of hobbies. However, every other method has the reversed parameter order -->memberOf('hobbies', 'photography')`.

Also, The code fails if the nested value is an array.

$collection = new ArrayCollection([
    ['name' => 'Bill', 'hobbies' => ['biking', 'running', 'swimming']],
    ['name' => 'Todd', 'hobbies' => ['video games', 'laser tag']],
    ['name' => 'Casey', 'hobbies' => ['tennis', 'golf', 'photography']]
]);

$criteria = Criteria::create()->where(Criteria::expr()->memberOf('photography', 'hobbies'));

$collection->matching($criteria)->toArray();

@ferjul17
Copy link
Contributor Author

Thanks for your feedback.

As you said I inverted the parameters in order to make it more readable because in my opinion this is the most important thing. But changing the order of the parameter takes about 2s with my IDE.

I fixed my code. Unfortunatly, it is not as beautifull as the rest of the code because I can not use the in_array function.

case Comparison::MEMBER_OF:
return function ($object) use ($field, $value) {
$fieldValues = ClosureExpressionVisitor::getObjectFieldValue($object, $field);
foreach ($fieldValues as $fieldValue) {
Copy link
Contributor

Choose a reason for hiding this comment

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

@ferjul17 Why can't you use in_array here ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mikeSimonson ClosureExpressionVisitor::getObjectFieldValue may return an instance of Iterator which doesn't work with in_array.

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Would you prefer something like that ?

@mikeSimonson mikeSimonson merged commit 5cea967 into doctrine:master Mar 27, 2016
@mikeSimonson
Copy link
Contributor

@ferjul17 Thanks

@ferjul17
Copy link
Contributor Author

You're welcome :)

2016-03-27 19:00 GMT+02:00 mikeSimonson notifications@github.com:

@ferjul17 https://github.com/ferjul17 Thanks


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#66 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants