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

CDbCriteria add new method createFindCommand #2659

Closed
Ragazzo opened this issue Jul 18, 2013 · 12 comments
Closed

CDbCriteria add new method createFindCommand #2659

Ragazzo opened this issue Jul 18, 2013 · 12 comments

Comments

@Ragazzo
Copy link

Ragazzo commented Jul 18, 2013

So as CDbCriteria acts like a query-object pattern and smth. like in Query in Yii2, i suggest to add new method to CDbCriteria, because of sometimes you dont need to use AR or populate models, just need simple command. So overall method can looks like this:

//CDbCriteria::createFindCommand($tableName)
return $this->getDbConnection()->getCommandBuilder()->createFindCommand($tableName, $this);

//CDbCriteria::getDbConnection
return Yii::app()->getComponent($this->connectionId); //also add here CDbConnection class check

Based on this also:
http://www.yiiframework.com/forum/index.php/topic/31900-create-a-cdbcommand-from-a-cdbcriteria/

@samdark
Copy link
Member

samdark commented Jul 18, 2013

I think it may be useful.

@cebe
Copy link
Member

cebe commented Jul 18, 2013

How do you think should the signature of the method look like? CDbCriteria itself is not aware of a database. This could be rather a method of CActiveRecord then and approach is similar to #819 / #531.

@Ragazzo
Copy link
Author

Ragazzo commented Jul 18, 2013

How do you think should the signature of the method look like? CDbCriteria itself is not aware of a database

described that in first post, read comments "//".

This could be rather a method of CActiveRecord then and approach is similar to #819 / #531.

as you see, those issues are closed and never will be implemented in Yii1 because of their difficulty, this approach is easy and useful shortcut, what is wrong? I understand that CDbCriteria acts like "lets replace array with value object and pass it into AR find methods". Also this can help in REST api.

@cebe
Copy link
Member

cebe commented Jul 18, 2013

described that in first post, read comments "//".

Okay, didn't get the full meaning of it as they where comments. So what you are proposing is this, right?

class CDbCriteria {
    // ...
    public function createFindCommand($tableName)
    {
        return $this->getDbConnection()->getCommandBuilder()->createFindCommand($tableName, $this);
    }

    public function getDbConnection()
    {
        return Yii::app()->getComponent($this->connectionId); //also add here CDbConnection class check
    ]

what is wrong?

Nothing's wrong I just wanted to mention that they are related. I am just not sure whether this should be a method of AR or CDbCriteria as the AR object has a criteria and also has a CDbConnection it might better be placed there.

An important thing to mention is that this method would not allow to do anything with relational query, so with and together properties will be ignored.

@Ragazzo
Copy link
Author

Ragazzo commented Jul 18, 2013

Okay, didn't get the full meaning of it as they where comments. So what you are proposing is this, right?

yes, but this is first sketch ofcourse, but is very simple and like 90% will be like this, yes.

CDbCriteria as the AR object has a criteria and also has a CDbConnection it might better be placed there.

no, this method is just for array data and simple select-commands that can be constructed "on the fly", nothing more, it should not belong to AR, because of CDbCriteria only serves as simple value-object for AR, so this method shoud not be there i think. But ofcourse other cases can be mentioned here and discussed.

so with and together properties will be ignored.

right, little price because of CDbCriteria was first design only for AR.

@Ragazzo
Copy link
Author

Ragazzo commented Jul 19, 2013

Or maybe we can add some new method to CDbCommand like applyCriteria so it will just fill needed fields with values from criteria.

@nineinchnick
Copy link
Contributor

I just added an issue exactly like this: #2872. I proposed to add a createCommand() in CActiveFinder. I think it resolves this issue too and also uses the 'with' property of CDbCriteria.

@Ragazzo
Copy link
Author

Ragazzo commented Sep 14, 2013

not sure if it solves this problem, but now i need to create CActiveFinder each time for that, but i want to avoid doing so, because this class is more internal then external.

@nineinchnick
Copy link
Contributor

A public method in CActiveFinder is needed anyway to be able to call it from outside. If you don't like the solution please propose a method in CActiveRecord that would wrap the example call I provided.

@Ragazzo
Copy link
Author

Ragazzo commented Sep 14, 2013

i dont think that this method must be in AR.

@nineinchnick
Copy link
Contributor

I'm sorry but your comments are not very constructive. Where should it be then? CDbCriteria always applies to a specific model when fetching data. It's database agnostic on it's own and to resolve joins it needs to be supplied with a db connection that holds a schema referenced in the joins.

@Ragazzo
Copy link
Author

Ragazzo commented Sep 14, 2013

I'm sorry but your comments are not very constructive

calm down, or i will be ignoring your messages because of personal attacks.

CDbCriteria always applies to a specific model when fetching data.

yes, it is so because of CDbCriteria acts like value-object, common way to replace array of params with value-object, thats why it is more used in AR. If you will look in Yii2 Query class, you will see that it is very similar to CDbCriteria and have method to create query from its own params, but not acts like simple value-object, because of it was good designed from the beginning unlike the CDbCriteria

About your method propose, yes it can be also in CActiveFinder for some features and workarounds, but i dont think that big number of developers will use internal class CActiveFinder for creating query from criteria. So this method can be in both CActiveFinder and CDbCriteria it is just suites different needs but very looks a like.

@samdark samdark closed this as completed Nov 17, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants