Skip to content
This repository has been archived by the owner on Nov 11, 2020. It is now read-only.

Make possible array argument for Query\Builder->select() #100

Closed
pierrre opened this issue Mar 27, 2013 · 3 comments · Fixed by #103
Closed

Make possible array argument for Query\Builder->select() #100

pierrre opened this issue Mar 27, 2013 · 3 comments · Fixed by #103

Comments

@pierrre
Copy link
Contributor

pierrre commented Mar 27, 2013

Currently Query\Builder->select() use multiple arguments (with func_get_args()).
It is not possible to give fields as an array and I think it's useful.

We should check if $fieldName is an array, and use elements of this array as field names.
If $fieldName is not an array, we use the current code (with func_get_args()).

The exclude() method should be also changed.

@pierrre
Copy link
Contributor Author

pierrre commented Mar 27, 2013

If it's ok, i can submit a pull request (with tests)

@jmikola
Copy link
Member

jmikola commented Mar 27, 2013

Is the use case that you have an array of fields and would like to avoiding using call_user_func_array()?

Since the field name should always be a string, I don't think this would cause a problem. Perhaps it might confuse someone accustomed to the driver's projection array (where the field names are keys and the values determine inclusion/exclusion).

If you do make this change, be sure to also update the exclude() method and include it in any unit tests. I believe those are the only methods that would need to be changed.

We do use func_get_args() in some other places, like the geo functions, but I don't think we'd want to touch those as it could get ambiguous (the arguments are already arrays).

@pierrre
Copy link
Contributor Author

pierrre commented Mar 29, 2013

Yes, I don't want to use call_user_func_array() with the query builder, because I can't chain calls.

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

Successfully merging a pull request may close this issue.

2 participants