apex-enterprise-patterns / fflib-apex-common

Common Apex Library supporting Apex Enterprise Patterns and much more!
BSD 3-Clause "New" or "Revised" License
899 stars 514 forks source link

Allow overriding the configureQueryFactory #464

Open wimvelzeboer opened 11 months ago

wimvelzeboer commented 11 months ago

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 commented 11 months ago

@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 commented 11 months ago

@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 commented 11 months ago

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 commented 11 months ago

@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');