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

Like or Not Like - that's where the question #50

Open
wants to merge 5 commits into
base: 2.0.x
Choose a base branch
from

Conversation

Fedik
Copy link

@Fedik Fedik commented Dec 10, 2014

New Comparison::LIKE and Comparison::NOTLIKE as alternative solution for #45 💃

Support next wildcard characters:
% - any character(s), any amount
_ - any character, only one
\% - escape for %
\_ - escape for _

$pattern = str_replace('%', '.*', $pattern);
$pattern = str_replace('_', '.{1}', $pattern);
$pattern = str_replace('SQLWILDCARDESCAPEDMANY', '\\%', $pattern);
$pattern = str_replace('SQLWILDCARDESCAPEDONE', '\\_', $pattern);
Copy link
Member

Choose a reason for hiding this comment

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

this implementation is flawed: it will break for strings containing SQLWILDCARDESCAPEDONE already.

Copy link
Author

Choose a reason for hiding this comment

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

true, but what can be alternative?
use DOCTRINECOLLECTIONSSQLWILDCARDESCAPEDMANY ? 😉

$pattern = str_replace('_', '.{1}', $pattern);
$pattern = str_replace('SQLWILDCARDESCAPEDMANY', '\\%', $pattern);
$pattern = str_replace('SQLWILDCARDESCAPEDONE', '\\_', $pattern);
$pattern = '/^' . $pattern . '$/';
Copy link
Member

Choose a reason for hiding this comment

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

Because of your other changes, some of my comments disappeared without being fixed.
All the pattern building should be done outside the closure given that it does not depend on the object. This will avoid rebuilding it for each object of the collection

Copy link
Author

Choose a reason for hiding this comment

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

disappeared, but I not forgot 😉

if I place it like:

$like = $comparison->getOperator() === Comparison::LIKE;
/// here the pattern building
return function ($object) use ($field, $value, $like) {

it will be good?
just not sure

Copy link
Member

Choose a reason for hiding this comment

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

yes it will

Copy link
Author

Choose a reason for hiding this comment

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

hope now is better 😄


if($pattern) {
return (bool) ($like ? preg_match($pattern, $fieldValue) : !preg_match($pattern, $fieldValue));
}
Copy link
Member

Choose a reason for hiding this comment

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

instead of checking whether a preg_match or an equality should be used each time the function is called, you could check it outside, returning different functions

Copy link
Author

Choose a reason for hiding this comment

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

yes, good idea

@Fedik
Copy link
Author

Fedik commented Dec 27, 2014

I just got idea, that we can use a random string instead of DOCTRINESQLWILDCARDESCAPEDMANY for placeholder
any tips about simple random string generator, or maybe Doctrine already has something?

@stof
Copy link
Member

stof commented Dec 29, 2014

Any random string will still suffer from the same issue. The difference is that you will not know what is the forbidden value

@Fedik
Copy link
Author

Fedik commented Sep 26, 2015

hm, hm, I wondered why it marked 🐛 ? I thought it is "new feature" 😄

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

Successfully merging this pull request may close these issues.

3 participants