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

Adding a more fuzzy search filter #16

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

sgtwinters
Copy link
Contributor

Like discussed in #15 hereby a suggestion to add a more fuzzy search filter to the CollectionOperator. It takes both the like and not like operators into account. It acts upon search filters with two asterisks (beginning and end).

@johannesschobel
Copy link
Owner

Hey man, thank you very much for the PR. Unfortunately, I am away the next days, so it may take some time to review the code.. hope this is ok..

Cheers

@sgtwinters
Copy link
Contributor Author

No problem, as soon as you got the time !

@johannesschobel
Copy link
Owner

I am just curious.. this looks kind of "black magic" here.. ;) Is it possible to make this "more readable"? Can we simplify both IF/ELSE branches in order to make this more accessible? What do you think?

@sgtwinters
Copy link
Contributor Author

Sorry for my slow reply, I was out as well. It indeed is a bit of black magic (encountered that too :P), but I kind-of followed the current implementation there: composing a rule (in this case a 2nd one) which will be evaluated. If we would like to improve I would say we should get rid of this method completely. I will try to look a bit further this weekend, but before I start: do you have suggestion which way to go?

@johannesschobel
Copy link
Owner

Actually i really like the way to filter based on specific fields. I think, that is quite handy and i would not like to remove it.

Maybe we can just add few more comments in order to make it better readable / understandable..

@sgtwinters
Copy link
Contributor Author

I agree that filter based on fields is perfect! I meant to say that filtering using the evals could change, but honestly, if we can clear things up a bit I'm happy too :D. I plan to use this package in a not-really-demanding environment, I think it suits such a case. In a high traffic case I would rather make use of Eloquent to perform first filtering and sorting (i.e. do more specific queries instead of grabbing full collections and filter those). I think that would be a more scalable approach (but will off course need a lot of work in this package) :). What do you think?

@johannesschobel
Copy link
Owner

I am completely fine with the current approach I initially chose for this package. I also agree, that you might want to do it differently in a more complex scenario. However, for smaller projects this is quite nice as it really removes a lot of work from the developer. ;)

@sgtwinters
Copy link
Contributor Author

Indeed! I will add comments to clear things up and than do a PR ok?

@johannesschobel
Copy link
Owner

you can add it to this PR.. just add more commits in order to enhance the currently open PR ;)

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.

2 participants