-
Notifications
You must be signed in to change notification settings - Fork 97
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
Added ability to init SearchQuery props on construction #546
base: main
Are you sure you want to change the base?
Added ability to init SearchQuery props on construction #546
Conversation
5f5ee10
to
71d2408
Compare
I added in the linter ignore for the Variable property access line. |
Is it really worth? With array you don't have autocompletion, because the structure is undefined |
I'd document the structure instead of ignoring |
71d2408
to
f9ba46f
Compare
I've updated the PR so all the properties are in the constructor. With PHP8+ you can specify the args using named arguments along with array spreading to get the same effect of setting all those properties quickly and easily using an array. |
int $hitsPerPage = null, | ||
int $page = null, | ||
) { | ||
foreach (get_defined_vars() as $name => $value) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Set the vars manually then the phpstan won't complain and in future it could be refactored to CPP
@Enchiridion do you still plan to work on this PR? 😊 |
Pull Request
What does this PR do?
This lets you init the
SearchQuery
props when the object is constructed. I think the PHP library is the only one that doesn't yet let you do this or use a simple array instead. I created this because I was using thesearch()
method but needed to switch tomultiSearch()
, but found it works completely differently, this speeds up using it.Previous way:
Additional way:
Note 1: My change currently does not pass the test due to the use of variable property access. However I think it's safe because all the props are simple and typed. If you think that's ok, I can have phpstan ignore that line.
Note 2: The initial code I based this on from
main
currently does not pass all the phpstan tests:PR checklist
Please check if your PR fulfills the following requirements: