Local classes are good

[sap]

Local classes are good

[HrFlorianHoffmann]

This section is the direct outcome of code reviews and expert discussions in our local group of three Scrum teams. For better understanding, here is the story.

We had one Scrum team that used local classes everywhere and for everything. All of their global classes would be only a couple of lines long and implement the factoy pattern:

CLASS some_class DEFINITION PUBLIC CREATE PRIVATE.
  PUBLIC SECTION.
    INTERFACES some_interface.
    CLASS-METHODS get_instance
      RETURNING
        VALUE(result) TYPE REF TO some_interface.
ENDCLASS.

CLASS some_class IMPLEMENTATION.

  METHOD get_instance.
    CASE requested_type.
      WHEN a.
        result = NEW lcl_variant_a( ).
      WHEN b.
        " ...
    ENDCASE.
  ENDMETHOD.

ENDCLASS.

The local class include would then contain up to ~12 local classes that served the factory variants, plus up to an additional ~10 classes and interfaces for utilities, enumerations, and other helpers. This local codebase usually amounted to several thousands lines of code. The corresponding unit tests were even longer, inflated by custom mocks.

The team's main argument was that this pattern enforces decoupling because consumers are literally unable to depend on concrete implementations. In their eyes, the global class was more like a package, less like a real class.

While there is truth in that, there is also truth in the consequences we observed:

• The code structure was obscure to outsiders, and backlog estimations for little changes regularly amounted to several weeks of effort.
• Code duplication was all over the place, because local classes by definition cannot call the local classes in other global classes.
• Orientation, navigation, and debugging in the thousands-of-lines includes was tedious and annoying.
• We observed "lock-wars", when several developers needed to work on different aspects of the same global class, because only one can lock the include for editing.
• Finally, we found that making everything local ignores the original idea of local classes as "highly specialized niche solutions of no value to others".

Refactoring this code to global classes improved our situation: estimations went down from ~100 days to ~30 days for the same things, several people could work in parallel on related parts, speeding up total implementation speed, code duplication dropped, and people just felt much more comfortable around the code.

[arcanist123]

Given the global namespace of SAP ABAP (no packages essentially)/30 character limit of class names / single responcibility principle I dont see how the local classes could be even avoided.

[vonglan]

Long discussion, but a point that I also found irritating (I have to admit that I only looked at the rules superficially so far).
I think it is important to acknowledge in the ABAP world that there are actually two different "worlds", in which sometimes different (even opposite) recommendations make sense:

• SAP internal or Third Party application / AddOn development
• SAP customer development (usually much smaller, often only little "deltas" to standard functionality, much shorter "release cycle")

It seems to me that the differences between Mike Pokraka's and Florian Hoffmann's opinion can very well be understood when you assume that Mike's work is more in the "customer development" world.

In the "customer development" world (in which I also live now, after about 7 years at SAP), I find that for more than 90 or even 95% of code, reuse does not make sense. In our main system, for example, I see multiple historic, ambitious efforts to create "reusable" classes like ZCL_M_MATERIAL. But when I look in detail (where-use list), I find that there is actually very little real reuse. Most global class methods are only used in exactly one report.

Therefore, in the "customer development" world, I think it is more important not to clutter the "PUBLIC" namespace with things that very very likely, no-one will ever want to reuse anyway.

(And in the exceptional case that you come upon a reuse case later on, it is easy to convert from local to global class.)

[HrFlorianHoffmann]

What you describe sounds like a more healthy use of local classes ("extract some private methods to own objects, e.g. to enable testing"). Nobody will object to that. So far, I'd say we have the same understanding and that it is more the formulation of the text (too harsh, doesn't make the points clear enough) we need to talk about?

[pokrakam]

I like a good discussion :-)

But I still don't agree. Your "at the moment" argument is one that can go both ways. Have a look at the idea behind YAGNI (grab a cup of coffee, worth taking the time to read and understand fully).

I can write a specific local class a lot quicker than a global, and - if it's well structured and clean - it's a doddle to extract and convert into a global class when the need arises. I've converted local classes to global many times, the basics take 5 minutes, the bigger effort comes in to make it "properly global" (or API-like if you wish), in other words generalising it and dealing with uncontrolled access. This is the potentially wasted effort that you'd have to put in up front, and it can be difficult to predict what need will arise if it doesn't exist at the time of coding so you might even cater for the wrong thing.

To use the Excel example, you illustrate my point quite nicely: which of the various scenarios you describe will come to be? Why spend effort in trying to cater for something that is not definite today?

So for low probability or unknown scenarios of re-use, let's not waste effort on someday-maybe and focus on the now. Modern development practices and editors make refactoring so, so much easier and safer than it used to be, and local classes are IMHO an important part of a flexible architecture. Global classes immediately make things more rigid, so I think local classes are worth considering when there is a high specificity and low probability of re-use.

But this all assumes modern development practices, ADT, unit testing and so on.

[cjbailey]

Given that the difference in effort of making a class global during initial build vs. afterwards is almost negligible, a bigger benefit is knowing the exact interface that is needed if we delay globalisation until re-use becomes reality. So in effect this saves time and simplifies things. At least that's been my experience.

Precisely. There have been times when I've started by making a global class only to find that I totally need to change my approach later on and the fact that everything is defined in the DDIC makes this more difficult. On the other hand I can start with a local class and easily transfer it to a global class later on.

as I mentioned it is also dangerous (global data / attributes)

No more dangerous than the things you can abuse with globally defined classes. At least with a local class that abuse is only going to have a localised effect.

To be clear there is a place and time for both. And as always, guidelines are guidelines, not an immutable truth.

[MCH123]

My humble opinion on this topic (sorry but I am strongly against local classes :p)

Example where I thought it was appropriate: a class creates a bespoke multi-worksheet Excel using ABAP2XLSX. A single entry point and a single result. Inner classes generate and render worksheets with headers, body and other elements. It's highly specific, not used by anyone else, and a single piece of functionality that one person will work on at a time.

The point is that its not used by anyone else at the moment... but could be in the future. Imagine that the business would want the same extract from another report ? Two similar layouts depending on authorizations ? What would you do ? rewrite a copy of your "highly specific" worksheet in another local class ?

To me the point of OO programming is to foresee and anticipate this as much as possible to allow reuse. If you are doing the job, why not doing it in a reusable manner while you are on it ?

Another scenario is classic dynpro reports using SALV in an MVC framework built from local classes (though the Model is often global).

From a maintenance point of view SALV should be banned as it does not covers all possibilities (if the business wants to add complex eventing by example you would have to redo it using CL_GUI_ALV_GRID).

Why not building a bespoke template class inheriting CL_GUI_ALV_GRID for the specific GRID part + an interface for data selection. Then when creating a new report, you inherits your template with your specific data structure + an ABAP class implementing your interface for the selection.

[pokrakam]

Hmm, are you not mixing ease of modification with ease of re-use?

A local class is certainly easier to modify, as you don't have the constraint of a public API and you have complete control over its usage. That's one of my main motivators for using them.

Putting effort into a building a public API on the assumption that someone may, in future, want to use a class differently = presumptive feature.

Also consider the XP and agile principle of Minimum Viable Product. Leave the bells and shiny lights until they're called for.

Given that the difference in effort of making a class global during initial build vs. afterwards is almost negligible, a bigger benefit is knowing the exact interface that is needed if we delay globalisation until re-use becomes reality. So in effect this saves time and simplifies things. At least that's been my experience.

Bear in mind, much of the earlier points were about localising highly specific functions - things that are related only to the application at hand or for other reasons should not be available externally (legacy code). So from the outset this discussion was mainly concerned with functions with a low probability of reuse. Which really makes it a presumptive feature.

PS. For what it's worth, the Excel example I mentioned was a Web Dynpro app with a download button that generated a workbook of multiple sheets with specific features including formats, protected cells, tables, formulas, clickable links to navigate around the spreadsheet. No real value in making that generic before we know what the next 'similar' usage might be, and certainly no off the shelf tool for it.

[pixelbaker]

Hi there. Thanks for the lively discussion.

I would use local classes more often if the ABAP Testdouble framework would support local interfaces, which it unfortunately does not.
That's why I am not making use of them at all.
Building my own testdoubles each time is too time consuming in comparison to using the ABAP Testdouble framework.
Having a unified way of mocking things also adds so much to a team, when maintaining each others code.

My two cents.

[pokrakam]

@HrFlorianHoffmann thanks for the valuable insights into how you guys work, I for one really appreciate it!

One thing I might clarify, when I talk about API, I do not mean it in the sense of "designed for reuse". It is just the way I view the distinction between public and protected/private components. Call it defensive coding if you wish - all public components form the API of a class. This viewpoint helps me to be more careful and mindful of scope and visibility at all times. This also relates to unit testing only public methods - I think of it as testing the class through its API.

Your description of "liquid" is spot on, that's exactly the analogy I was looking for. But I am surprised that people make access difficult, I must say it is not something I've experienced much.
I would welcome someone taking a local helper class I wrote and turning it into a more generalised global class. To me this would be YAGNI at work: I built what I needed, the next person needs something similar and extends it with an API (in the re-usability sense). Similar effort, no guesswork, the end result suits both of our use cases, win-win.

[HrFlorianHoffmann]

Suggest to close this then. @pokrakam and @mAltr in case you disagree just ping me and I'll reopen.

[vonglan]

@HrFlorianHoffmann :
I don't know whether I can still technically add anything (and whether someone reads it ...), but I would like to reiterate my view from above (different "world" SAP vs. customer) in the light of @nununos statement. His points are valid from a SAP or Third-Party-Developer point of view. But in the customer context, I do not think that the package encapsulation is an effective way to improve quality. And I never had to create global includes (for constants that are needed in multiple frame programs, I use global interfaces - special case).
In my opinion, local classes are the best default in the "customer world".

I did not yet look into all rules here in detail. But I could imagine that there are more rules which differ for SAP/ThirdParty vs. Customer. And I think it would be great if we make this difference explicit instead of being irritated with each others opinions in the discussions.

[pokrakam]

Yes, I do think it's harshly formulated, it suggests that locals are all but verboten. "Take care when using local classes" is an approach I'd be comfortable with. Your explanation would be a great addition to this section.

I fully understand your scenario: the global class is the facade and the application is hidden within. I am aware of this danger - and guilty of it myself on occasion too. Sometimes it is justified, often it should not be.

Example where I thought it was appropriate: a class creates a bespoke multi-worksheet Excel using ABAP2XLSX. A single entry point and a single result. Inner classes generate and render worksheets with headers, body and other elements. It's highly specific, not used by anyone else, and a single piece of functionality that one person will work on at a time.

Another scenario is classic dynpro reports using SALV in an MVC framework built from local classes (though the Model is often global).

Writing this, I realise that specificity is an important factor to consider.

[HrFlorianHoffmann]

Let me reopen this as an anchor to think about some better text formulation and maybe add some details from our story. For example, I think the lock-wars thin isn't even addressed as of now.

[pokrakam]

Sorry I didn't even notice I closed it by accident.

[HrFlorianHoffmann]

Started a pull request with a suggestion how to rewrite the section to accomodate the findings and suggestions from above.

[pokrakam]

Hi Florian,

Overall very well explained. However I feel that this bit is too subjective:

Orientation, navigation, and debugging in very long local class includes
is tedious and annoying.

I don't disagree, but it can go both ways which is why I find "tedious and annoying" subjective. Two counter-examples:

From personal experience at the other extreme, I've worked on a customer project that took the single responsibility principle to the extreme and favoured 'micro classes' (many with as few as 5-10 statements), resulting in literally thousands of global classes. At the end of a debugging session I'd have 30-40 tabs open in Eclipse and had no idea where up or down was anymore. The call stack functions perfectly well through both local or global classes, so to me debugging is not an issue.

Secondly, I use Ctrl-O to navigate in Eclipse and it works the same for me whether I'm in a large locals section, a global class, or a DPC class with over 10k LoC in hundreds of methods. In some instances it actually makes it easier: What if I know the method and can't remember which class it belongs to? Ctrl-O finds it in a local class or report include. Or a local interface with several implementations, Ctrl-O lists all the classes that implement a particular method and lets me choose the one I'm after. So there are some pluses to navigation in local classes too.

Perhaps write something along the lines of:
Consider the impact of long source code segments on orientation, navigation and debugging. If the source code is too large, it could be a sign of low cohesion, and it may be a good idea to split some local classes and their dependents out into a new global class.

Just as an idea, another angle could be reports. For an ALV report (SALV), I'll typically create several view and controller classes and a variety of support functions. The only global classes are the model classes. Not sure if you want to include such an example?

[MCH123]

I also have to add that local classes are highly dangerous as you can directly use global data from the report, select options, parameters... bypassing the visibility. I agree that it should not be done this way but I am seeing this too often.

[cjbailey]

The point is that its not used by anyone else at the moment... but could be in the future. Imagine that the business would want the same extract from another report

Don't future proof code - a scenario like this would be a good opportunity to refactor your existing code while moving it to the data dictionary. You have no idea at the time of writing if another business unit would want exactly the same functionality or something slightly different. Trying to build a one-size-fits-all class to begin with could leave you with some bad design decisions that are harder to unwind later on.

[MCH123]

Oh please come on... there is a difference between making something clean and reusable and making it future proof (which is not possible).

Dont exaggerate :)

a scenario like this would be a good opportunity to refactor your existing code while moving it to the data dictionary

It is the same that if I replied "ok so lets hardcode everything and it will be a good opportunity to refactor later" ;)

[cjbailey]

It is the same that if I replied "ok so lets hardcode everything and it will be a good opportunity to refactor later" ;)

Now who's exaggerating?

If you genuinely believe that something will be of use elsewhere then sure make it a global class, but this does not mean that all classes are equivalent and most definitely does not mean we should never make local classes.

[MCH123]

Now who's exaggerating?

that was my point

[MCH123]

Have a look at the idea behind YAGNI (grab a cup of coffee, worth taking the time to read and understand fully).

I read the link about YAGNI, thanks for it I didnt know about it and I think it is interesting. I quite agree with the autor. I think there is even a sentence which summarize what I am talking bout :

> Yagni only applies to capabilities built into the software to support a presumptive feature, it does not apply to effort to make the software easier to modify

I never talked about building presumptive features but at least making it possible.

To use the Excel example, you illustrate my point quite nicely: which of the various scenarios you describe will come to be? Why spend effort in trying to cater for something that is not definite today?

You are just making it easier to modify and evolve by building it in a global API like class.
You could also go futher if needed : I already saw a template Excel development which is reading customizing table to build the Excel layout. Any user can create/modify their layout and style using a transaction.

[MCH123]

Hmm, are you not mixing ease of modification with ease of re-use?

Closely linked in my opinion. If it is well built, it is easier to modify and to re-use.

A local class is certainly easier to modify, as you don't have the constraint of a public API and you have complete control over its usage. That's one of my main motivators for using them.

Not that much easier and as I mentioned it is also dangerous (global data / attributes). To me it is almost the same than using form routines in this aspect. Plus building a global class doesn't force you to support all use cases either.

I understand your point of view about building local class for highly specific functions but I still feel that it is more because you are confortable with it rather than it being the cleanest solution.

[pokrakam]

Closely linked in my opinion. If it is well built, it is easier to modify and to re-use.

Hmm, I'm not convinced, I've seen plenty of coding horror behind a perfectly usable API and vice versa (why else would so many developers write their own BOPF wrapper? :-0 )

I don't see global data as a danger specific to local classes either. A global singleton class is essentially the same thing, and plenty of people mis/use those.

[cjbailey]

@pixelbaker I have to admit, I'd never heard of ABAP Testdouble framework before your post, thanks for the info! After a cursory glance over it, it seems like SAP's take on the Moq framework. Nice!

[pokrakam]

@pixelbaker interesting point.

While TDF is a useful framework, I think it's a bit heavy duty and can be overkill at times. See Writing simple, readable unit tests. TDF would just add an unnecessary layer in this scenario.

But I agree if you have a class that would benefit from TDF then that's a good argument for making it global.

[HrFlorianHoffmann]

@cjbailey Clean ABAP actually recommends to use the ABAP test doubles.

While @pokrakam's has a point that cl_abap_testdouble is overkill for simple input-to-output calculations, and can be quite verbose in its setups, it is usually still preferrable to requiring the reader to get familiar with own, custom doubles.

@pixelbaker We found it a worthwhile compromise to make the interfaces global, while using local classes to implement them. The interfaces alone are usually worthless, such that the probability that somebody outside will use them is negligible.

[HrFlorianHoffmann]

A lot of this discussion revolves around when we should design something for reuse ("API style"), and when we shouldn't ("private" / "local"). This question involves a lot more than only whether to make a class local or global. For example, we encounter the same line of thought in the section FINAL if not designed for inheritance. Should we make the you-ain't-gonna-need-it principle more prominent, both in the local classes section, and the guide overall?

[HrFlorianHoffmann]

Another story from our internal discussions. In our daily work, we often encountered code that we would have liked to reuse, but couldn't, because it was heavily safeguarded. Local classes, glocal friends, and private constructors prevented instantiating classes, while in-line instantiation of dependencies instead of dependency inversion made it nearly impossible to disentangle the safeguarded classes.

Our teams appreciated the notion of making clear that something is not good enough yet for reuse, or that it should not be used elsewhere because it was still too "liquid". However, they also found it a little disquieting to what considerable lengths some people would go to actively prevent others from reusing their code.

In an expert discussion, the majority preferred to keep the code global and accessible, with little to no active restrictions who may see and construct the classes. They agreed that we should prefer other constructs to discourage usage, such as package interfaces, some "API" property on objects, or some variant of "! DO NOT USE annotation.

[pixelbaker]

@pixelbaker We found it a worthwhile compromise to make the interfaces global, while using local classes to implement them. The interfaces alone are usually worthless, such that the probability that somebody outside will use them is negligible.

@HrFlorianHoffmann That's an excellent idea. I like to give that a shot and see how that approach goes. Thanks for sharing Florian.

[f4abap]

I have now worked through the whole discussion and hey, that's a lively party. Could also be a good blog on SAP Community...
anyhow, at the end I have to add my thinking here and hey, sorry for all the local class lovers.
I don't see any reason to have a local class, execpt the statement I want to use make it necessary. I don't see an advantage of a local class.
IF a global class is not reused or even cannot reused, where's the problem. It's just nearly the same code at another place on the storage.. but if there is a day and I need to add or reuse something I'm always open to anything in mind when used something global...
So of course all the arguments with the visibility and all others I can understand, but do not agree from my past experiences.

[mAltr]

I have to strongly agree with @pokrakam and use local classes for specific casses. This is the only way, to keep a class privat.
The same discussion would be, why to use "private" methods,... it´s simply a part of information hidding. In our case, the class itself is the information.

[nununo]

While I understand @pokrakam's argument for local classes, here are 3 arguments against it:

1. This repository should be showing the proper way
You acknowledge that packages are the right way to do it but dismiss them for not being widely adopted. This repository is trying to define what the best practices are. So that is what it should be: a reference on how things should be done. If, instead, its suggestions are patched to adapt to how people work... it won't ever helping them improve, and package encapsulation will never be widely adopted.

2. Less global classes = more global includes
I am currently working on a program which has dozens of classes. It is very specific and I don't expect any of these to be reused. So, according to your logic... they should all be local. Even the one called from the SE38 report.
I agree that ideally these should no be made available to others, but I also feel that the ABAP development environment and its user interface (ADT for Eclipse included) are not designed to favour local classes.
Because... in order to avoid dozens of global classes I'll need to create dozens of (global) includes, one per class, with even more obscure names. And then I'd have to figure out where to include each one of them and that would be a mess because the include command in ABAP doesn't behave like in java.

3. Local classes cannot be grouped into subdomains
This program I'm working on is subdivided into several subpackages, properly encapsulated. This is only possible because I'm using global classes. Were I to use local classes, it would be impossible to group them into these logical subdomains and the program's high level architecture would be much harder to understand and therefore it would be much harder maintain.
So, by avoiding packages due to lack of adoption I'd be delivering poorer code because I would not be able to profit from what they have to offer.

[HrFlorianHoffmann]

Is there still a contradiction, @mAltr? The section suggests to use global classes as default, and local classes only where appropriate. This seems to fit to your statement "use local classes for specific casses".

[mAltr]

@HrFlorianHoffmann, the current wording is fine for me. I hope that it won´t change in the future :)

[vonglan]

I would add to the current text:
Local classes are suited

• When reuse of the code is very unlikely, for example in a typical SAP customer situation with mostly small development topics

[Suncatcher]

The local class include would then contain up to ~12 local classes that served the factory variants, plus up to an additional ~10 classes and interfaces for utilities, enumerations, and other helpers. This local codebase usually amounted to several thousands lines of code. The corresponding unit tests were even longer, inflated by custom mocks.

The team's main argument was that this pattern enforces decoupling because consumers are literally unable to depend on concrete implementations. In their eyes, the global class was more like a package, less like a real class.

As I got from this Florian's description this team put all the functionality here?

and only used global class for calling local? This is really freaky and odd and I never seen such usage, it shouldn't be done ever