Unclear explanation of [Use LOCAL FRIENDS to access the dependency-inverting constructor]

[ConjuringCoffee]

The section "Use LOCAL FRIENDS to access the dependency-inverting constructor" is unclear to me. It currently only contains this title and a code example.

The previous chapter "Use dependency inversion to inject test doubles" explained to use a constructor to which the dependencies are handed over. Nowhere does it say that the constructor should be private.

Is the LOCAL FRIENDS section talking about dependency-inversion using a factory class instead? Or is it talking about another form of dependency inversion I'm not aware of? Either way, I think the section would benefit from a better explanation.

(The code example also has syntax errors regarding the class name which we should clean up as well.)

[N2oB6n-SAP]

As a prerequisite for this discussion please be aware of the Open SAP course Writing testable code for ABAP and its ideas of "constructor injection", "setter injection", and "backdoor injection". You can check this module for the presentation of these concepts.

The clean ABAP styleguide has a strong opinion on which techniques to prefer and which techniques to avoid (e.g. setter injection). In my personal opinion all injection approaches may be valid depending on the context, most importantly green field vs. legacy code, as long as you are aware of the implications.

I agree that a couple of sentences with the reasoning behind the recommendation of couldn't hurt here given my interpretation of Use LOCAL FRIENDS to access the dependency-inverting constructor:

Applying the dependency inversion principle is generally a good idea. It's part of the famous SOLID after all and it greatly increases the testability of your code. While the guide argues that the "FRIENDS"/"backdoor" injection technique can be considered harmful the LOCAL FRIENDS trick can still be reused for constructor injections. When controlling instance creation with lightweight factory methods you should declare CREATE PRIVATE so consumers are forced to use the factory method. By using LOCAL FRIENDS your local test classes are able to circumvent the factory method and inject appropriate mocks. Thereby you keep the public API unchanged and do not increase its surface. (To apply this idea to proper factories you need to use global test classes and link them with GLOBAL FRIENDS.)

That's my best guess about the intention of that section. You are right about syntax error, of course. We can fix it with the yet-to-be-created PR that provides some explanation for this section (...once we've settled on its meaning, relevance, ...).

[ConjuringCoffee]

I think that's the missing link: The style guide implies the use of factory methods when it talks about the "dependency-inverting constructor". To me, a "dependency-inverting constructor" refers to "constructor injection" where the constructor has import parameters for its dependencies.

I've checked the slides from "Writing testable code" and it doesn't seem to mention that constructor injections should be accompanied by factory methods. Why would you have factory methods as part of dependency-inversion? Is it just because you don't want the caller to bother with optional parameters in the constructor? Or maybe if you don't want the caller to be able to supply the dependencies...? Factory methods aren't really compatible with inheritance in ABAP, so I generally avoid factory methods.

[ConjuringCoffee]

Björn mentioned some interesting thoughts on the topic of factory methods in: SAP/abap-cleaner#104 (comment)

[ConjuringCoffee]

Based on the existing input, I suggest the following changes to the style guide:

• Suggestion to consider using a factory method returning the interface and making the class CREATE PRIVATE. This could be included as a new section in "Classes".
• Suggestion to consider using a factory method in the context of dependency-inversion: Instead of having the dependencies optionally in the constructor, make them mandatory in the constructor and create them in the factory method. This could be a new section in "Testing" or an addition to the existing section "Use dependency inversion to inject test doubles".
• Rephrase section "Use LOCAL FRIENDS to access the dependency-inverting constructor" to not be about the dependency-inverting constructor, but simply about the class being CREATE PRIVATE.

@bjoern-jueliger-sap, does this match your thoughts?

[bjoern-jueliger-sap]

The general architectural pattern that the style guide seems to implicitly assume but not explain here is the following:

This part is explicit: Classes should have no public instance methods that are not part of an interface and "the dependency-inverting constructor" should be accessed via LOCAL FRIENDS.

This part is implicit: Classes should be CREATE PRIVATE and offer factory methods whose returning parameter is typed with the interface of the class. The private constructor of the class should have all dependencies as importing parameters, while the factory methods should have only those appropriate for the use case for which the factory method is intended.

For instance, applications often depend on some sort of local configuration which you need to dependency-inject into order to test them, but in all production use cases this will be some sort of default configuration read from the database of the system, so there is no value in forcing callers of the factory method to supply this dependency themselves. Here's a small example that hopefully illustrates the pattern:

class cl_app definition final create private.
  public section.
    interfaces if_app.
    
    class-methods start_app
      returning value(app) type ref to if_app.
      
    methods constructor
      importing config type ref to if_config.
  private section.
    data config type ref to if_config.
endclass.

Then, in the factory method we do

  method start_app.
    app = new cl_app( cl_config=>default( ) ).
  endmethod.

so no external caller needs to know how to get the configuration instance, but in our unit tests we use LOCAL FRIENDS to supply a mock config to the private constructor instead of the productive configuration in the system.

I can see that this style does not come "naturally" to ABAP developers and agree that the explanation in the guide is unclear.