Some doubts regarding apex best practices and SOLID principles

Question

I have a lightning component controller class:

public class ComponentVisibilityController {
        @AuraEnabled
        public static Boolean checkToDisplayComponent(Id recordId) {
            return ComponentVisibilityFacade.run(new LeadComponentVisibility(recordId)); 
        }
    }

In the ComponentVisibilityFacade class:

public class ComponentVisibilityFacade {
        public static Boolean run(ComponentVisibility componentVisibility) { //LeadComponentVisibility is a Child of ComponentVisibility. 
            componentVisibility.setCurrentUser();
            .
            .
            .
            .
            .
            return componentVisibility.displayUnshared();
        }
    }

In the ComponentVisibility class I have the below method and more:

public virtual class ComponentVisibility {
        protected User currentUser = null;

        public void setCurrentUser() {
                currentUser = UserDetailsUtil.queryUserDetails(userInfo.getUserId());
                System.debug('currentUser: ' + currentUser.Name);
        }
            
        public Boolean displayUnshared() {
            .
            .
            .
        }
    }

As you can see UserDetailsUtil is static class that calls queryUserDetails method and returns the current user.

I have other Util classes (most of them just query various objects and return results) that are all static and are called in LeadComponentVisibility class.

So in summary:

• A static class ComponentVisibilityController (I know the class can instantiated but since it only has a static method, I am not going to instantiate it, hence a static class) is calling another static class ComponentVisibilityFacade, where a new instance of LeadComponentVisibility (dynamic binding) is created which calls another static class UserDetailsUtil (via inheritance) to get current user.

Now:

1. Should I make UserDetailsUtil an instantiable class? Or is it ok I leave it static? I mean is there a problem with multiple instances of LeadComponentVisibility class calling the static UserDetailsUtil?
2. Is my design correct? That is, I am following good software design principles(SOLID)? Or am I missing something or can the class structures be improved? For example: UserDetailsUtil just queries user object based on the user or lead owner id I am passing and does nothing else(the S in SOLID).
3. LeadComponentVisibility decides whether to make a component visible or not but also calls other classes and their methods because I need to decide whether a component is visible based on current users and owners roles, hence setCurrentUser method and others. Is calling these classes and their methods in LeadComponentVisibility OK or am I violating the S?
4. Since this is Util class if today itself I end up using UserDetailsUtil.queryUserDetails(Id user) method in another class that requires another field. What should I do? Since this field is used only in this another class and not in ComponentVisibility class, I am not unnecessarily querying a field I don't need?
5. In UserDetailsUtil.queryUserDetails(Id user), I am querying only required fields of User. But tomorrow (say a year later) if I need another field of the user for the same ComponentVisibility class, how would I do that? Yes I know I can simply modify the query. But this violates the O in SOLID. Should I query all the fields so that I don't violate the O? Again, I am not unnecessarily querying fields I don't need?

Answer 1

SOLID is a great principle to live by, in theory, but Apex has many practical limitations that require you to make pragmatic decisions that will violate the "letter" of SOLID. It's more important to live by the "spirit" of the policies as much as practical. SOLID is now 20 years old, and it definitely wasn't written with Apex (or any limited CPU/memory/query) language in mind.

Since we have governor limits, it's usually more important to remember YAGNI (You Ain't Gonna Need It) and KISS (Keep It Simple, S<redacted>). If you don't need instance variables or multiple copies, make your methods static. That's not really even a problem with SOLID; it says nothing about not using static methods.

A class may call other classes in order to maintain the Single Responsibility Principle. The "S" does not state that a class must do everything itself, simply that it should do just one thing. It should control a Visualforce page, or serve a REST response, or handle calculating sharing permissions for a user. It should not do all of those things in the same class. In particular, a child class should call its parent methods as necessary to help it out, otherwise it has little business being a child of that class.

The "O" has an interesting point. If you have a method queryUserDetails, but those details may vary, you should either have multiple methods, or simply query the records when necessary. You don't need to consolidate every single instance of a query into a method for reuse. It's okay to have similar queries in different places, especially if they'll never be called in the same transaction. Don't beat yourself up over it.

In regards to your last point, have you ever wondered why there's no "SELECT * FROM OBJECT" as there is in SQL? It's because Salesforce wants you to consider each query you write carefully to avoid excessive heap usage. It's a very limited resource, and a wildcard-like query could easily hit the governor limit with just a few rows of data. If you can't aggregate your queries into a single method, just use more than one, or use it on demand.

If you don't understand SOLID, don't worry about it too much. Honestly, most of these principles will come to you with experience, if you even need them. "S" is pretty common-sense, "O" will usually "mostly" occur normally, and if you have to violate it, you'll know why, "L" is rarely violated, as you have to either intentionally do so, or be so inexperienced that you've never used subclasses more than about three times, "I" is rare to even need, but if you do, you'll find it's actually the path of least resistance, and "D" is not particularly used in most business logic, so by the time you need it, you'll probably have the experience to use it responsibly.

My only recommendation is to ditch the queryUserDetails method until you actually find a common need for it. If queryUserDetails does something else (which might be a violation of "S"), have it accept a list of users and do what it needs to do (e.g. query roles, calculate sharing, etc). Each caller can then do the query it needs without worrying about the "O" principle. Alternatively, write different versions of the method, or have it accept a list of fields that the caller wants.

As far as static methods, as long as you don't need instance variables or method overrides, feel free to make them static. That's what static is meant to be used for. SOLID doesn't actually prohibit static methods anyways (it says nothing at all about it).