Make params to Model::find() easier to type? #401

Closed
al-the-x opened this Issue Mar 28, 2012 · 5 comments

4 participants

@al-the-x

Please consider the following amended signature of Model::find():

public static function find($type, array $conditions = array(), array $options = array())

Then finders could be used as follows:

SomeModel::find('first', array(
    'some_model_id' => 1,
));

SomeModel::count(array(
    'some_other_field' => 'some value',
));

SomeModel::all(array(
    'some_nullable_field' => null,
));

This would be preferable to typing "conditions" all the time, but it would significantly change the API for Model. I realize that's a big ask, but I think it's a better API. If this is an acceptable change, I'd be happy to adjust the code and tests and submit a pull request. Thanks,

David @ OrlandoPHP.org

@nateabele
Union of RAD member

Like you say, it's pretty close to impossible to do this without breaking every find call ever, which is a huge barrier for a change like this. That said, there are plenty of ways to write queries without having to type 'conditions' all over the place. For example, primary key shortcuts like Posts::find("4f737f4c7675abe113000000"), or magic finders like Posts::findAllByStatus("published").

Failing that, you can always just write a custom finder that maps whatever custom values you want to your conditions.

@davidgolding

Ah, such is the beauty of the aspect-oriented paradigm. Worth keeping it the way it is since there's a robust filter capability for model finds in Lithium. (My 2¢.)

@al-the-x

The trouble comes into play when you have multiple fields that need finding, e.g.

SomeModel::first(array('conditions' => array(
    'some_model_field' => 'some value',
    'some_nullable_field' => null,
)));

I can't easily add a finder that does what I would like (subclassing works just fine). If you're opposed to changing the signature for Model::find(), what about adding code to it that passes along all passed parameters (via func_get_args() for example) to the filters. Then I could add a finder that would do what I want all aspect-friendly and stuff.

@nateabele
Union of RAD member

I see nothing wrong with creating a base model that does what you want. That sort of modification isn't strictly filter-appropriate, anyway.

@davidpersson
Union of RAD member

I'm closing this ticket which I labeled as a RFC now. Thanks @al-the-x for initially opening this ticket with a detailed explanation and offering your help! Implementing the suggested signature would be a huge API break for the (pre-)1.x Lithium series also alternatives as @nateabele suggested exist.

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