Skip to content

findByXXX does not accept orderBy/limit/offset#115

Closed
mpdeimos wants to merge 1 commit into2.1.xfrom
unknown repository
Closed

findByXXX does not accept orderBy/limit/offset#115
mpdeimos wants to merge 1 commit into2.1.xfrom
unknown repository

Conversation

@mpdeimos
Copy link
Contributor

@mpdeimos mpdeimos commented Sep 2, 2011

Our modification to pass findByXXX functions orderBy/limit/offset params for entity repositories like one is used from normal findBy function.

We're using an if cascade in favor of the call_user_function_array since the latter is considered as slow.

PS: If i need to target this pull request to your master, I'll resubmit - it's just that we're building against a stable codebase.

@guilhermeblanco
Copy link
Member

Idea seems very great, but your code needs some polish. @beberlei could you please look into this one?

@guilhermeblanco
Copy link
Member

I created an issue for this one: http://www.doctrine-project.org/jira/browse/DDC-1426

@mpdeimos
Copy link
Contributor Author

thx, (w/o looking at the code) a better solution for the if-cascade might be always calling the 4 param method and pass dummy params (null/array()) to indicate missing arguments of findByXXX

@asm89
Copy link
Member

asm89 commented Oct 17, 2011

If we introduce this new functionality, there should also be tests for it.

@asm89
Copy link
Member

asm89 commented Mar 11, 2012

Merged and added tests here 39ad876 and 3faa1a7.

Thanks!

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.

3 participants