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

Allow overriding the configureQueryFactory #464

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

wimvelzeboer
Copy link
Contributor

@wimvelzeboer wimvelzeboer commented Sep 2, 2023

I would like to write my own Selector factory that makes a few minor changes to how it communicates with the QueryFactory.
To do that I would like to change the configureQueryFactory method into protected and virtual.


This change is Reviewable

@ImJohnMDaniel
Copy link
Contributor

@wimvelzeboer -- Just a question regarding this PR. You mention that you want to change the way that the fflib_SObjectSelector communicates with the fflib_QueryFactory. Can you elaborate on the intended changes and why you are not including changes to fflib_QueryFactory as well?

@wimvelzeboer
Copy link
Contributor Author

@ImJohnMDaniel I would like to add a method to the selector class, something like setLimit, but then I need to change the configureQueryFactory method a tittle bit so that I can pass that to the query factory.

A while back I tried to add that functionality to the selector class in another PR but after a discussion it was declined. I can understand that, but still want to have that for my own project, so I was thinking about extending the selector and add that functionality. That is however only possible if this method is virtual.

I would like to do something like:

List<Account> records = 
        new AccountsSelector()
                .selectByRating('Hot')
                .setLimit(100)
                .setOffset(200);

This way of calling a selector is particular useful for a controller class that is using pagination. In this way the selectByRating method can be re-used by other parts of the application that do not require a limit/offset.

@daveespo
Copy link
Contributor

Isn't what you're proposing for a sample use case a very significant change in expected behavior? Specifically, all selectBy... methods in a Selector are expected to return a collection of records.

Your sample is implying that the setLimit() and setOffset() will be applied to the SOQL generation -- which means that the query execution will be deferred until .. when?

@wimvelzeboer
Copy link
Contributor Author

@daveespo you are right, the example is in the wrong order. It should be:

List<Account> records = 
        new AccountsSelector()
                .setLimit(100)
                .setOffset(200)
                .selectByRating('Hot');

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.

None yet

3 participants