Can I use a Selector in a controller method directly, or is it required to call a Service that is going to call the selector?

[AllanOricil]

Can I use a Selector in a controller method directly, or is it required to call a Service that is going to call the selector? Is there a technical problem? or is it just a recommendation to respect "SoC"? I fell that creating a service method just to call a selector disrespects "KISS". So what the maintainers think about it?

Let me give an example. Currently I have this controller method which just calls a Selector

@AuraEnabled
    public static List<ContentDocumentLink> fetchContentDocumentLinks(Id linkedEntityId) {
        try {
            return DSP_ContentDocumentLinksSelector.newInstance()
                .selectAllInLinkedEntityIds(new Set<Id>{ linkedEntityId });
        } catch (Exception e) {
            throw new AuraHandledException(e.getMessage());
        }
    }

Now, according to "SoC" I actually "must" call a service class that will call the selector. By doing so, I will end up with these additional classes and interface, just to accomplish the same thing:

1 - DSP_ContentDocumentLinksService 2 - DSP_ContentDocumentLinksServiceImpl 3 - DSP_IContentDocumentLinks

I had to write a lot more :/

[stohn777]

Hi @AllanOricil

Of course nothing technically prevents the usage of a Selector in a controller. However the application architect must continually weigh the costs and benefits of the current design principles versus the likelihood of change versus the cost of redesign later.

Micro-service architectures are more expensive upfront but offer greatly flexibility as the application evolves, IF the evolution aligns with the design. Monolithic designs are cheaper upfront but may incur great cost as the business and application matures.

Every architect must consider the business's perspective on cost, risk, schedule and change, which are only as good as today's information. You'll not get a DO or DO NOT answer from many in this circle on the matter but more questions regarding the principles that are guiding the design of the application.

[AllanOricil]

@stohn777 although there are no "DO" or "DO NOT" answer I just want to point out that having a Service method that only calls a Selector method brings an unnecessary overhead to the Service class, to the project, and to the developer.

1 - Service classes will get gigantic if we do it for every single selector method. 2 - The project (fflib app) can end up with Lots of Service classes that are only used as Adaptors to call that single Selector method (bad practice according to kiss) 3 - Developers will have to edit multiple classes to maintain a service method that only calls a selector method. I mean, imagine a situation where that selector method is no longer necessary. In this case the developer will have to remove that method from 4 different places: selector class, service class, service implementation and service interface.

Considering those 3 arguments above, and that there are no rules, I think in this case we should actually point out about this overhead at least in the docs and add a section "what you should not do". This way we can avoid having discussions about it.

One architect on my team used this image to argument that it was necessary to call a service method to call that single selector method, and I understand why he did it, but I disagree because of the reasons I mentioned above, and because this image say nothing about a selector specifically.

Moreover, because Im no authority in fflib yet, It is better to discuss with people who are activelly using it so I can learn more.

[AllanOricil]

For me, Service methods should be for things that have more complexity than just calling a selector method, otherwise people will always use that image above to justify every single decision instead of measure/mitigate trade-offs for taking decisions.

[wimvelzeboer]

I get this question a lot @AllanOricil My rule of thumb is that it is OK to directly call the Selector from the Controller class (and other classes like in the image you shared), if your intention is to take the records from the selector and basically return it in your controller without any modification or filtering.

So your example would be perfectly OK:

@AuraEnabled
    public static List<ContentDocumentLink> fetchContentDocumentLinks(Id linkedEntityId) 
    {
        try 
        {
            return DSP_ContentDocumentLinksSelector.newInstance()
                .selectAllInLinkedEntityIds(new Set<Id>{ linkedEntityId });
        } catch (Exception e) {
            throw new AuraHandledException(e.getMessage());
        }
    }

But I would advise using a Service when your intent is to modify field values or filter records (if not yet done in the selector itself). Something like the following should be refactored to using a Service method, in my opinion :

@AuraEnabled
    public static List<String> getAccountNamesOfHotUsaAccounts(accountsIds) 
    {
        try 
        {
            return 
                Accounts.newInstance(accountsIds)
                    .selectByCountry('USA')
                    .selectByRating('Hot')
                    .getAccountNames();
        } catch (Exception e) {
            throw new AuraHandledException(e.getMessage());
        }
    }

[AllanOricil]

@wimvelzeboer You code example could be declared in the Selector as a method:

public List<Account> selectAccountsWithNamesOfHotUsaAccounts(){
    return (List<Account>) Database.query(
        newQueryFactory(false)
        .selectField(Account.Id)
        .selectField(Account.Name)
        .setCondition('Country= 'USA')
        .setCondition('Rating = 'Hot')
        .toSOQL()
    );
}

And then you could call it from the Controller directly

@AuraEnabled
    public static List<String> getAccountNamesOfHotUsaAccounts(accountsIds) 
    {
        try 
        {
            return 
                AccountsSelector.newInstance().selectAccountsWithNamesOfHotUsaAccounts();
        } catch (Exception e) {
            throw new AuraHandledException(e.getMessage());
        }
    }

For me, services are more for things like

public List<Opportunity> doSomething(){

List<Account> accounts = AccountsSelector.selectAccountsWithNamesOfHotUsaAccounts();

Accounts accountsDomain = (Accounts) Application.Domain.newInstance( accounts);

Set<Id> someListOfLookupIds = accountsDomain.getSomeLookupListOfIds();

return OpportunitiesSelector.newInstance()
                    .selectByListOfLookupIds(someListOfLookupIds);
}

Something that requires some sort of domain manipulation and/or more than one select more than once. Like, "there are lots of things" happening inside the service that if we had used all these lines of code in more than one place, we would have to do multiple updates in more than one place in case this logic/code changes, wich would require lot of rework. And because nobody wants to do that, a Service method would be justified in this case. But when it is just a Select, I really don't see any reason for spending time writting a Service method that only "wraps" a Selector method.

It is basically the main reason for why we create "functions". Why do we create functions? To avoid repeating code and change things in more than once place. To centralize the logic when it is something more complex that is needed in other parts of our software.

[ImJohnMDaniel]

@AllanOricil, I agree with @wimvelzeboer in that selectors can be called from the client tier (e.g. controllers) as long as you are just doing what needs to be done to supply the records to the LWC/Aura/VFPage or whatever client you are dealing with.

Services in the FFLIB / Apex Enterprise Patterns world tend to be more "coarse grain" services. If the logic crosses multiple SObjects, then it is an obvious need for a service method. Domains tend to be SObject specific or other discretely focused elements. Services usually orchestrate the movement of domains. As @stohn777 and @wimvelzeboer both mentioned above, if you plan to do any business logic or DML actions on the records, then that is when a service should take over. Again a general rule would be that business logic should never be in any client tier elements.

[AllanOricil]

@ImJohnMDaniel but which parts do you disagree with what I said? Can you explain why and where you disagree, pointing out why you wouldn;t do such thing.

[ImJohnMDaniel]

@AllanOricil, maybe I am missing something here. Can you elaborate please?

[AllanOricil]

Never mind, I understood @wimvelzeboer code now. I did not read that he said to refactor the code snippet he posted above. I thought he was saying to write that snippet.

Yes, I agree with that.

We should always refactor something like this:

@AuraEnabled
    public static List<String> getAccountNamesOfHotUsaAccounts(accountsIds) 
    {
        try 
        {
            return 
                Accounts.newInstance(accountsIds)
                    .selectByCountry('USA')
                    .selectByRating('Hot')
                    .getAccountNames();
        } catch (Exception e) {
            throw new AuraHandledException(e.getMessage());
        }
    }

to this:

Selector

//the query fields Country and Rating are to exemplify only. I dont really know if they exist 
public List<Account> selectAccountsWithNamesOfHotUsaAccounts(Set<Id> ids){
    return (List<Account>) Database.query(
        newQueryFactory(false)
        .selectField(Account.Id)
        .selectField(Account.Name)
        .setCondition('Country= 'USA')
        .setCondition('Rating = 'Hot')
        .setCondition('Id IN :ids')
        .toSOQL()
    );
}

Service

    public List<String> getAccountNamesOfHotUsaAccounts(Set<Id> accountsIds) 
    {
        try 
        {
            List<Accounts> accounts = AccountsSelector.newInstace().selectAccountsWithNamesOfHotUsaAccounts(accountsIds);

            return  Accounts.newInstance(accounts).getAccountNames();
        } catch (Exception e) {
            throw new DeveloperException(e.getMessage());
        }
    }

Controller

@AuraEnabled
    public static List<String> getAccountNamesOfHotUsaAccounts(accountsIds) 
    {
        try 
        {
             return AccountsService.getAccountNamesOfHotUsaAccounts(accountIds);
        } catch (Exception e) {
            throw new AuraHandledException(e.getMessage());
        }
    }

because in "this case" we needed to transform/filter data like @wimvelzeboer said

[AllanOricil]

I'm closing this as it is now clear that there is no problem in calling a selector method directly from a Controller. Thank you again for sharing your thoughts :D

[wimvelzeboer]

@AllanOricil I would like to make one change in your service logic:

public List getAccountNamesOfHotUsaAccounts(Set accountsIds) { try { List accounts = AccountsSelector.newInstace().selectAccountsWithNamesOfHotUsaAccounts(accountsIds);
return Accounts.newInstance(accounts).getAccountNames(); } catch (Exception e) { throw new DeveloperException(e.getMessage()); } }

It's better not to use an AuraHandeledException in a Service class. Either use an exception specific for the Service class, or no try-catch at all. AuraHandledException wouldn't make any sense when the service method is invoked from the context of a Batch Apex Job..

[AllanOricil]

yes, sorry. I just copied and paste code haha. I will edit so other that read it here don't get lost

[cropredyHelix]

@AllanOricil

shouldn't this line:

List<Accounts> accounts = AccountsSelector.newInstace().selectAccountsWithNamesOfHotUsaAccounts(accountsIds);

be?

List<Account> accounts = AccountsSelector.newInstance().selectAccountsWithNamesOfHotUsaAccounts(accountsIds);

that is, the selector returns a list of SObjects, not a list of domains

[AllanOricil]

Yes, you are right.