Enhancing a dinosaur – clean(?) modification of SAP objects with “USEREXIT” Forms

[CreyJ]

Assuming you have the chance to start up wit a new ERP or S/4 SAP system - yes they are also still alive in an S/4 Hana -  and you can work on an untouched SAPMV45A (as example). How would you do it? What is clean (for you)?

Here are some of my thoughts – but no final conclusion. Maybe you can add yours.

• Direct implementation in ZZ include:
  
  
  
  • Pros
    
    
    
    • Direct access to all data objects
    
    
    • Easiest way
    
    
    
    
  
  
  • Cons
    
    
    
    • Complete include locked by one change
    
    
    • No unit tests
    
    
    • Direct access to all data objects, when doing it wrong (ok that's always the case when doing it wrong, but ... )
    
    
    
    
  
  
  
  

• Direct implementation using includes, one (or more) per USEREXIT form :
  Similar like direct implementation
  
  
  
  • Pros
    
    
    
    • several developers/projects can work in parallel
    
    
    
    
  
  
  • Cons
    
    
    
    • Looks a little bit like fake modularization, as include is not an own object
    
    
    • Tendency to grow wild. “May-be another include in or after the original include helps me with my problem?”
    
    
    • A return or check statement  leaves the subroutine and not just the include
    
    
    
    
  
  
  
  


• External performs to programs, that share the data object with tables/common part
  
  
  
  • Pros:
    
    
    
    • Still direct access to data
    
    
    • Can have own screens
    
    
    • Implementation could be done by local classes and unit tests
    
    
    
    
  
  
  • Cons
    
    
    
    • External performs look like a misuse of the private objects, that are public by technical reasons.
    
    
    
    
  
  
  
  

• Real OO with global interfaces/classes/BAdis
  
  
  
  • Pros
    
    
    
    • Advantages of OO: Encapsulation, Inheritance ..
    
    
    • “Modern” Exception handling
    
    
    • Development support by ADT
    
    
    • Unit tests
    
    
    
    
  
  
  • Cons
    
    
    
    • Cannot have screens
    
    
    • Frequent changes of public interface when starting with a reduced/minimal interface
    
    
    
    
  
  
  • Variants
    
    
    
    • One class/interface/BADI per main program / one instance per document/transaction
      
      
      
      • Pros
        
        
        
        • Can have own attributes / member variables
        
        
        
        
      
      
      • Cons
        
        
        
        • May result in large classes/interfaces
        
        
        • more complicated to handle, see also https://github.com/SAP/styleguides/blob/main/clean-abap/CleanABAP.md#make-singletons-only-where-mult...
        
        
        
        
      
      
      
      
    
    
    • One class/interface/BADI per USEREXIT form
    
    
    
    
  
  
  
  


• How to perform the modification
  
  
  
  • Modification of the SAP include
    
    
    
    • Pros
      
      
      
      • Easier to transport in an running system
      
      
      
      
    
    
    • Cons
      
      
      
      • Needs a registry key
      
      
      • More complicated in SPAU
      
      
      
      
    
    
    
    
  
  
  • Implicit enhancement
    
    
    
    • Pros
      
      
      
      • No key needed
      
      
      • Easier in SPAU
      
      
      • Can have a different transport route than a modified SAP object
        e.g. when applying notes in a central development system
      
      
      
      
    
    
    • Cons
      
      
      
      • Import of an enhancement creates a new runtime object. So it cannot be imported that easy in a running system.
      
      
      
      
    
    
    
    
  
  
  
  

 

Currently my thoughts go in direction of having a program that shares the data with the master program. The call from the USEREXIT form is done by external subroutine call:

E.g. like this for USEREXIT_FIELD_MODIFICATION.

perform field_modification in program <your program> if found.

Starting point for the implementation of the custom code would be a local class, as there are easy refactoring paths to  global classes.

The modification would be done using an implicit enhancement.

Let me know your thoughts.

[BaerbelWinkler]

jrgen.creyaufmller

Hi Jürgen!

Thanks for your thoughts about how best to enhance SAPMV45A, a discussion we've also been having for several years without any clear/clean solution (see my question from 2017). We are currently restarting our internal discussions, so your blog post is very timely! We'll have to wrestle with a rather large (i.e. 35,000 lines of code) existing enhancement, so obviously quite far removed from a clean slate or fresh start!

Right now, we have almost all of the code in one big enhancement with some code put into includes to allow for parallel development for country specific logic where one include per relevant country had been added to each USEREXIT-Form-Routine.

One often voiced con-statement to breaking things up into e.g. BAdI-Implementations (or even just includes) is that it's then no longer possible to do a version compare for "everything" in the SAPMV45A enhancement in one go. Using a function group might help with this, I guess.

I can't quite picture how "a function group that shares the data with the master program" would look like on the object level, though. Could you please clarify?

Thanks much and Cheers

Bärbel

[CreyJ]

Hi Bärbel,

when you use TABLES statement or COMMON PART a data definition, you use the obsolete, but still living interface work areas. "All programs in a program group access the same data of an interface work area." You just have to grant that your program is only bound to SAPMV45A.

So you just a have to add the same tables and common part definition of SAPMV45A in your program; the easiest is to include these includes of SAPMV45A directly.

I just reread the common part docu and it does not work with a function group, so I will change my initial post.

Regards

Jürgen

 

[CreyJ]

Another thing to version comparison. If you use one package for SAPMV45A enhancements, you could use TA SREPO to compare everything between two systems in almost one go. At least it tells you in the first step all different objects.
If you want to compare to older versions, you could use ABAPGit.

[Sandra_Rossi]

I would do it directly the right way, by definining a Z BAdI definition ("real OO"). One method per USEREXIT subroutine (only the ones you are interested in, of course). Define the parameters that you need (all those TABLES, COMMON PART and so on).

One BAdI definition for all USEREXIT subroutines of one program. One BAdI implementation per subroutine (or more if your company is a complex organization requiring one or more BAdI filters).

NB: screens is not a CONS as you can easily call a function module from your BAdI implementation.

 

[CreyJ]

Hi Sandra,

Thanks for your reply.

When using one BAdI with one method per USEREXIT routine– keeping example of SAPMV45A  - the BAdI/Interface gets quite big. When applying a filter like VKORG VTWEG VBTYP you can have many implementations besides the fallback implementation. As soon as implementations are create, the fallback isn’t called anymore and you always have to keep all methods of all implementing classes up to date. Inheritance can help, but as soon a re-definition is done, this method may need additional maintenance effort in future. And as several implementations may be valid, someone has to care about “once and only once” execution.

When using one BAdI for one  USEREXIT routine this problem is not that big, but it is still existing. When looking at e,g, at USEREXIT_SAVE_DOCUMENT, there is quite a lot of code with many different features inside. Trying to put the feature switches to the filter creates a very big filter. And adding a new feature needs to rework filter and existing implementations.

What to you think about a “Feature switch customizing” that enables you to switch features on and off or a defined default behavior. Customizing is much more easy to transport to a running system than transporting a complete enhancement implementation. Global interfaces or public sections also cannot be imported that easy w/o problems to a running production system, as this will cause dumps when the interface/class is already loaded. And this is likely the case for SAPMV45A.

Regards

Jürgen

[former_member610590]

column-table string hash should resovle any issue with filter.

have used it for a long time without problem.

what does it mean:

TABLE with 3 columns:
1) filter
2) filter-pos
3) filter-values (type char 200)

so you have always one filter, but which could combine any SD-characteristics

filter id provide you with target class.

[vonglan]

Hi, we reorganized our MV45AFZZ 3 years ago. I copy my header comment here (re-formatted), which summarizes our approach and design decisions. Feel free to ask questions!

INTRODUCTION:
After many years of historical changes to MV45AFZZ and related SAPMV45A INCLUDEs (plus the modifications in MV45AF0B_BELEG_SICHERN), and many years of developers’ complaints about the resulting spaghetti code, we decided to move all the included customer coding (12,000 lines) to global classes with the 2018 system upgrade.

HOW TO FIND YOUR WAY AROUND:

• The original 49 INCLUDE programs have been deleted. They can however be accessed, by calling SE38 and navigating directly to the version history via the menu
• All new classes are prefixed ZCL_VAX_... (Vertriebsauftrag Exits) and are assigned to package ZV_SALES_ORDER_EXITS (can be searched using report RS_ABAP_SOURCE_SCAN or transaction CODE_SCANNER)
• Usually, there is one ZCL_VAX class per User Exit. However, sometimes exits with very little customer logic have been combined. And some topics are so big (e.g. UNIQUE_PLANT) that they have their own ZCL_VAX class
• The names of METHODs are identical to the names of FORMs (minus the prefix)
• Variable names have not been changed (except when in conflict with naming conventions, e.g. for parameters)
• Code itself has not been changed (exceptions see below)
• At the beginning of classes and methods, there are references to the original INCLUDE, with year and author

HOW TO ADD NEW FUNCTIONALITY:

• If possible, add a call to a new private METHOD in the ZCL_VAX...=>MAIN methods
• In case of locking problems (or expected locking problems):
  see below
• keep the method interface as small as possible. E.g. if only VBAK-VKORG is needed, use a parameter I_VBAK_VKORG
• try to keep the filter logic (e.g. on AUART) in the calling method
• If Integration PERFORMs are needed, also try keeping them in the caller
• User MV45AIZZ for PAI-, and MV45AOZZ for PBO-modules
• Avoid global variables!!! If you need them for use on screens/dynpros, use include MV45ATZZ, and prefix them with ZZ

GOALS AND PRINCIPLES OF THE MV45AFZZ REDESIGN:

Improve transparency

• by moving code from FORMs (which are obsolete) to global classes (and subsequently using interface parameters instead of global variables)
• by keeping the parameter interfaces as small/narrow as possible
• by moving filter logic to the caller, so it is immediately evident whether some logic is relevant or not (without having to navigate to deeper levels)

As far as possible, leave coding in the FORMs/METHODs as it is

• To help with re-orientation
• To avoid new errors

Exceptions (code changes)

• Obsolete language constructs (leading to syntax errors when used in METHODs)
  • Especially, the short form of LOOP statement with header tables was changed to LOOP i_it_xvbap REFERENCE INTO DATA(xvbap)
  • Reasons:
    • REFERENCE (instead of field-symbol): because field-symbols are less well supported by Eclipse (refactoring)
    • xvbap (instead of wa_xvbap): same name as before, fewer changes necessary, anyway mixing up is impossible because the tables have _it_ in their names
• ATC Prio 1 and 2 findings resolved (like with any new coding)
• Filter logic moved to the caller (see above)
• Integration PERFORMs (like PERFORM vbap_bearbeiten_ende IN PROGRAM sapfv45p.) moved to caller if easily possible
• Irritating comments / tags removed in extreme cases
• unnecessary CLEAR command for local variable at the beginning of procedures removed
• layout of SQL commands adapted in extreme cases (e.g. indentation of AND further left that SELECT)

For significant changes or comments, the tag "mc18" = MV45AFZZ cleanup 2018 has been used

ATTENTION:

In general, keep in mind that the compiler sees changes to header comments of global classes as changes to the public section of the class. So re-generation of the sources is enforced during import. In some cases, it can make sense to move the “header comment” to an alternative position (implementation of constructor).

WHAT TO DO IN CASE OF LOCKING PROBLEMS:

If the program object that you need to work on is currently locked by a different RFC (development topic):

• Contact the other developer, to check if there are underlying conflicts
• Use a _temporary_ workaround:
  • temporary reset to a historic version
  • use an implicit enhancement
  • use INCLUDE IF FOUND in the main() method of the correct zcl_vax_... class
  • use a different position (e.g. directly in MV45AFZZ instead of the correct position in ZCL_VAX...=>MAIN)
• Remove the temporary workaround afterwards!

[hardyp180]

Just as a warning SAP at one point a year or so ago said that the PERFORM IF FOUND was going to cause a hard syntax error in "customer" code. Good job too as it is an abomination. I have seen so many short dumps because someone changed the signature of the remote FORM routine.

They (SAP) may have backed down on this, but a BADI is still the best way. There are tons of BADIS for the SAPMV45A transactions.

The thing about BADIS if they are configured a certain way then the standard SAP code loops through all the implementations so many developers can work on different changes at the same time.

Many people do not realise this but there is nothing at all to stop you creating your own BADI and putting it in one of the SAPMV45A ZZ user exit FORM routines and then you can have 25 different developers working on this at once for 25 different requirements.

Fun fact - look at SAPMV45A in SE38 and look at the "attributes" - you will see it is not set up for decimal places!

[hardyp180]

And in regard to the COMMON PART thing that is the worst nightmare I have ever seen. I presume it was invented a a cure for a problem that no longer exists.

[abo]

The amount of  c r a p  that I see in this include across customer installations is mindboggling.

And more than a bit disheartening.

[vonglan]

One specific comment: I would avoid sharing all of MV45A's data (via TABLES or COMMON PART) with your new (restructured) code". That only continues the spaghetti code paradigm.

Of course, it is more effort to pass in the required variables. And sometimes (rarely) you need to change the signature.

But the advantage of clear interfaces (in a general sense) is much bigger: that you can see immediately which structures/tables a method IMPORTS, CHANGES etc.

 

About Z-BAdIs: Sounds like a good idea. I am not sure though whether it is too much effort when you want to convert your legacy MV45AFZZ code (versus starting on a green field). Does anyone have experience with this?

Using the process described in my comment above, we "converted" about 100-200 legacy lines of code per hour.

[matt]

If you think MV45A is awful - have a look at Bex variables some time. The same approach of using BADIs is the only sensible solution. I installed such a framework at one client. A subclass per variable. Worked nicely, no more locking of the exit by a developer.

[Sandra_Rossi]

Sorry, didn't see your reply (please use "reply" button so that people are informed).

Big? How many USEREXIT routines did you implement. Anyway, what issue does it make to have a big interface?

Concerning the way how to organize the code through filters or feature switch, I'm pretty sure there are as many solutions as SD implementations.

I think it's more an organization/responsibility problem than a technical problem (using BAdI should be the first choice, but you can do your own solution too).

I can't help you choose the best solution, it depends too much on your USEREXIT routine implementations.

[vonglan]

Not sure what you mean with "Bex variables". The BW Tool?

Could you also elaborate on "subclass per variable"?

Regarding "No more locking of the exit": Locking also has advantages, when you have too many external developers: the system makes you aware that someone else is working in the same area, so you can manage that, instead of blindly running into trouble because of side-effects.

[matt]

BEX is a BW tool yes. You can create your own variables. The CMOD user exit handles any new variables. If you put the logic directly in it, and someone else needs to add a variable for their own product, the object can be locked against a transport that isn't associated with theirs. Or you can end up with overtaking transports and you can't get needed changes productive.

Which makes it very hard to manage,  which kind outweighs any perceived advantages.

In any case, by coding directly in the includesall the variables are essentially tightly coupled. Far better to use a BADI solution which decouples dependencies and provides sensible modularisation and encapsulation.

Includes should never be seen as a good way to modularise code.

[mmcisme1]

😉  And none of that  was written by a consultant they had?

[abo]

It's difficult enough to keep your own house in order; when you find that the situation is already compromised, if you're a good guy you try at least to leave it a little better than what you found. And that's a big IF 😉