Skip to Content
Technical Articles

Refactoring an ABAP Include Monolith

Motivation

Usually, in R/3 you see a common pattern: Spaghetti-Code within of program includes of a certain function group. Especially, in case of extensions of SAP standard this was required and caused the creation of big monoliths, whereby more than 10.000 lines of code was not rarely.

In addition, another problem as consequence is that only one developer could do a change of the source code, because the whole function group was locked. I would also assume that those changes could not be tested very well. Therefore, the developer would not be aware of any side effects.

This document tries to help you solving that problem.

Objectives

I would like to define the following objectives:

  • Code has to be changeable
  • The developers have to be enabled to implement requirements w/o any risk of side effects to existing functionality and also to work in parallel
  • Not all user have to be affected by the refactoring è reduce risk of complete failure

Non-Objectives:

  • The creation of micro services
  • Leverage (S/4)HANA features
  • Only refactor code, which is really being used (see SCMON/simplification process)

Context

  • Users (also machines, sensors) use the coding in production. We have to be careful!
  • Probably, the quality of the source code is very low. I would expect there:
    • Bad identifiers for variables/constants
    • Commented coding
    • Literals
    • Spaghetti-Code / redundant coding
    • Call of subroutines (PERFORM)
    • Use of “dirty-assign” to other function groups
    • Direct database accesses

Procedure

Apparently, step „zero“ is to get the permission and the budget/time for such a big refactoring.

Also mission-critical is the moment to begin with the refactoring: I would recommend starting before the conversion to S/4HANA, because the conversion could rise syntax errors or other incompatibilities. Unfortunately, not every feature of ABAP unit will be supported on lower NetWeaver releases. However, if you could transfer the old coding w/o any problems to S/4HANA, you can start there as well.

Generally, I recommend as IDE the ABAP Development Tools (ADT) to perform the refactoring, as it is superior to classic SAP GUI.

  1. Freeze coding: Only very urgent hotfixes are allowed. Further development is prohibited
  2. Create a backup: You should create a snapshot of the current coding (e.g. via git). With that you have a “plan B” and maybe can go back, if something goes completely wrong
  3. Do simple refactoring: Rename variables, introduce constants, delete commented code à should be possible w/o any risk
  4. Prepare test data:
    1. Create a testgroup with appropriate test user: In the best case, you have a wide diversity and involve several departments of the company to achieve a good test data quality
    2. Create a clone of the include in an ABAP OO class (let’s call it simply “agent”)
      1. Provide a simple entry method, like “execute” to call it from the original function group
      2. Encapsulate all dependent data of the original function group. I would recommend to create a dedicated class for that (name: “Origin Function Group API”)
      3. Calls to local subroutines also have to be extracted to the Origin Function Group API. Reason: Probably they would operate on the data of the function group
    3. Encapsulate external accesses in the agent: Calls of the same function modules (incl. dirty assigns) and access to database tables should be refactored into appropriate classes. Let us call them “DB Access”, respectively “Other Function Modules API”.
      In order to avoid a direct coupling you should work with ABAP OO interface. You could also create the instances of the classes by using a “Dependency Lookup”
    4. Integrate a “Test Spy” into the classes: The spy is an additional class, which logs the input- and output parameter of the method calls. I would recommend to use one Spy for all classes, because you can also log the sequence of method calls. The log data should be stored into particular database tables or in an eCATT-test data container; I prefer the eCATT-container.
    5. Integrate agent into origin function group with a user parameter:
      1. Once the users of the test group have activated their user parameter, they should do their usual business
      2. In addition I also recommend to activate the code coverage analyzer for the test group in order to find also some blind spots
      3. Dependent of the program include, you should record the test data appropriately. For example, if the coding is only relevant at year end closing, you should consider this time period in your recording
      4. Until now, some coding already has been changed, so there could be a small risk, that you have made some mistakes. Therefore, the test group should also keep an eye on the functional correctness. Of course, in case of any errors, they have to be fixed immediately.

Intermediate result:

  • There is no chaotic read to the function group’s data. Instead you use an API
  • Also other accesses (database tables, other function modules) are (well) defined now
  • You are able to create test data for the automatic testing. This is the fundament for a more complex refactoring, which we want to perform in the next steps

Class diagram (high level):

Hint: During the recording, you as a developer could already think about, how to split this big agent class into appropriate business function classes. Surely, you could start with the first test data a prototype in a local copy of the agent. (see also the next steps)

  1. Provide unit test infrastructure for the agent class: Dependencies have to be „mocked“ and provided with the data from the test spy (expected input/output data, calling sequence)
  2. The unit test should be successfully at the first time: If not, maybe you need to adjust the unit test infrastructure or one of the other classes
  3. Start the complex refactoring: Extract method/classes (see also „Separation of Concerns“-principle). You should also use ABAP OO interfaces to decouple from the concrete classes. Recommendation: Start first with extracting some method. By the name of the methods, you could see if there are common aspects (e.g. country specific) and extract them to new classes, so called business functions
  4. Extract parts of the “global” test data from the spy to the business function: At the agent, all test data for all business functions are maintained, but this raises also the complexity. Moreover, you would not be able to implement new requirements in parallel. Therefore, you have to extract the relevant test data also the business functions unit test. Usually, the redundant test data is obsolete and you can remove them from the eCATT test data container.
  5. The agent only orchestrates calls to the business functions: The unit test infrastructure should only test the agent, not the business functions itself
  6. Adapt test spy, if required. I recommend, to create after step 5 to create a new test data container, as a preparation for the next phase
  7. Extend your test user group with further participants / departments: Now, you can act iteratively and repeat the relevant steps from before
  8. Finalize refactoring and clean up: After the previous step, where you could extend the test group as long as you would like to do (dependent on risk estimation, personal security feeling, time), you have to remove the test spies. At this moment, you do not record any test data. Moreover, you have to activate the new logic for all user

Final Result

  • Classes with clear defined responsibilities: Better to understand the coding
  • Modularization allows testing with ABAP Unit: Fearless change of existing code
  • Further development in parallel mode possible due to separation of concerns into multiple OO classes
  • Also an additional benefit: Eases conversion to S/4HANA due to a better code base, because of automated tests, understandable coding and elimination of redundant code

 

What do you think? Do you have other ideas/best practices? Please share your remarks in the comments. Thanks a lot.

21 Comments
You must be Logged on to comment or reply to a post.
      • Hi Sandra,

         

        To me the biggest difference is, refactoring is a development technique, not a project. It happens in an ongoing basis. If you need permission and budget to do so, you are in a project (or work order or whatever) and it is definitely not refactoring.

        With all that said, I really like the approach, very detailed and I will definitely consult it when/if I need to rewrite 🙂 a monolith.

         

        Cheers,

        Ciustodio

         

        PS: There are obviously a lot on this subject on the web. Two of my favorites are https://techbeacon.com/app-dev-testing/rewrites-vs-refactoring-17-essential-reads-developers and https://blog.ndepend.com/rewrite-or-refactor/

         

        • Thank you for the detailed explanation and links. I was not aware of the importance of what is behind those words, although I understand “refactor” in TDD. Maybe the term “restructuring” as used in the first article (among other possible terms like “redesigning”, “reworking”, …) is better than “rewriting”, if one considers that “rewriting” is creating code from scratch without looking at the old code.

      • Refactoring is rewriting the code without changing the logic or logical flow. E.g.

        • Renaming variables.
        • Adding modularisation by breaking up a method into submethods.
        • Ensuring the correct visibility of variables – such as ones not really global
        • Extracting an interface from a class, and define all instance variables with reference to the interface
        • Changing the code so it can be ABAP unit tested. (Putting it in a test harness).
        • Changing a function module controlled ALV to a CL_SALV_TABLE.

        But there’s no hard line, I think. It’s a continuum. On the left, definitely refactoring, on the right, definitely rewriting.

        Where I’ve had to enhance an existing FORM based programs, I’ve shifted the FORMs into methods, and then enhanced. That’s refactoring at first.

        When I’ve had to change the logic of an existing FORM based programs, I’ve just rewritten it.

    • Hi Custodio,

       

      Thanks for your feedback.

      I would prefer the term “refactoring”, because “refactoring” is for me an iterative approach to restructure my code. The approach described here, is a “complex” refactoring, because also the structure would be changed as a final result – all this wouldn’t be happen at once, but in small steps.

      The term “rewrite” would mean for me to start with a blank page. Of course, you also can do this, but that seems to me very risky and hard to meet the requirements from the last decades w/o having enough tests.

       

      Kind regards,

      Stefan

      • I have recently rewritten an ABAP Objects program, where correct design was lost. I used the original to define the functionality, then used TDD to get that functionality in the new program.

        We were able to test the new and make sure we got the same results as the old. Then I fixed a few issues and added new functionality to the new.

        The key thing was that we did have enough tests.

  • Hi Stefan,

    that is interesting: 13 likes, but only 1 comment so far.

    It seems the topic is regarded as important, but there is not much to say/comment about it.

    During our last upgrade, I refactored the sales order user exits (MV45AFZZ) in our system.

    We chose to do this during the upgrade, because this minimizes the amount of additional (manual) testing.

    What I did was simply to transfer all the coding from FORMs in INCLUDEs to methods in global classes.

    So we got rid of the global variable access, and the obsolete language constructs that are allowed in non-OO code. I also solved all ATC priority 1+2 findings.

    I did not add any unit tests. And I kept all the routine and parameter names, and most of the comments, like they were, so all developers that previously worked on the exits could still find their way around.

    One advantage is that when you have to add or change something in those exits now, the risk of unwanted side effects is smaller because there are no global variables any more.

    Another unexpected advantage is, that we can now transport small changes to the exits during working hours without this leading to MISMATCH dumps (as long as the changes are class-internal and do not affect the public section).

    Greetings,

    Edo

    • Hi Edo,

      looks as if I’ll have to pick your brains a bit about this one of these days as we “should” also do something comparable with this exit/enhancement! Unfortunately, we haven’t made any progress since I asked – and closed – my related question in March 2007……

      Cheers

      Bärbel

    • Hi Edo,

       

      Thanks for your feedback and sharing your experiences. Basically, that was also my idea to extract the logic in to separate classes 🙂

      Maybe, one additional questions: How could you make sure that your changes were valid? Did you have some test users or was it at risk?

       

      Thank you.

       

      Kind regards,

      Stefan

      • Hi Stefan,

        just noticed that you’re also in the DSAG ATC author team 🙂

        Regarding testing: we did this as one part of our last system upgrade (suggested by the development team). During the upgrade, we have a big (manual) business user test anyway, so that was our safety net.

        Cheers,

        Edo

    • “And I kept …  most of the comments, like they were, so all developers that previously worked on the exits could still find their way around.”

      I’ve often been able to remove comments by encapsulating the code the comment refers to in a suitably named method.

      * Now we flibble the autofrumps
      LOOP AT blah ...
        flibble.
        flibble.
      ENDLOOP.

      becomes

      flibble_the_autofrumps( blah ).
      • Hi Matthew,

        I like that approach.

        However, due to time constraints (I had to transform those 10,000 lines between the “normal” upgrade tasks) and again not to completely alienate previous developers, I only did this for recognizable chunks of at least 50 lines of code.

        Best regards,

        Edo

  • Very good summary, thank you! This definitely supports the “Boy Scout Rule”.

    I think this might make the topic more accessible and move it from “impossible, we can never do this” to “well, why don’t we try?”…

     

     

     

     

  • I don’t think you will in general be able to do that kind of refactoring . It’s too large a change for an activity that is mostly likely viewed as nice to have.

    It’s correct, I just don’t feel it’s realistic . A more affordable refactor will just remove individual method calls from the include into classes so that when you change a line of code you don’t have to transport the entire include (and reduce locking problems). Thats easier to explain to management , not so easy to explain all the test code, business class refactoring, etc.

    • Hello Joao,

       

      Thanks for your remarks. I understand your concerns – yes, the described approach is complete guide to get rid of the monolith and maybe you are not allowed to do all of that. Surely, you can use parts of it, like just extracting some logic into classes. But, how do you know, that your functionality still works? How do you react on code changes?

      It always depends what you want to achieve: If you just want to reduce the locking problems and missing tests is not an issue, then you just can extract/introduce classes. IMHO missing tests are always an issue, if you want to change your software afterwards.

      If you cannot change your software anymore (and w/o interrupting your production), I would not consider that as a “nice to have”. Unfortunately, it is sometimes hard to explain that to the management, till it is too late.

  • I call it a rewrite as well.  During an “upgrade” – we didn’t bring everything over to S/4 nor did we rewrite what we did.  Time lines are so very important.  We found that we really had to work hard to get the information into our new system for training.  Normal issue came up that needed addressed.

    As Joao Sousa said above, I don’t think it is realistic.  It’s just too much changing.  Then once we start to learn to integrate the new tools that HANA brings us, another rewrite would have to happen.

    I don’t disagree that it would be nice to have.  However, that’s where it would fall in priority, nice to have.

    • Hello Michelle,

      Please have a look at my answer above 🙂

      I’m not sure, if I would compare the transformation from R/3 to S/4HANA with the big includes in customer systems. Moreover, for SAP it is not so easy to record the appropriate test data with all the variants you have. Maybe you could go for your internal tests and demos as a base in order to make sure your functionality still works.

      I would like to know, how would you make sure that your changes wouldn’t affect the customer systems? Or do you really do a rewrite and substitute the old logic/tool with the new one after you think you are done?

      Like I wrote above: It is so long a “low priority/nice to have” till it is too late – code needs to be changeable (in an efficient matter of time)…

      • Sigh.  I wish we had the time.  What we did was pull over our SAP that we needed.  We knew it was not going to be in SAP because it was custom to us.  Or when we were testing we found out that we needed something.

        So no rewrite for us – most of the time.  It was a customer solution that was outside of SAP.  The hooks did need to change for where it went into our SAP system.

        The rewrites were ones that took advantage of our new system.  Those were done prior to our final push.  And yes, they were rewrites.  There is a lot of differences between 4.6C technology and NW 7.5.  Some of them went really  fast.  Most of the rewrites were done by consultants that knew the new system better.  However, some of them needed to be fixed later.   When you rewrite something you are bound to do something wrong.  But we didn’t take much with us.

        Testing…  Testing and more testing.  The complete system was tested as it was basically a new install.

        Lots of work – but it was interesting and fun getting the new technology.

        And what we brought over – we could rewrite it.  BUT the code we brought over is critical code.  Not changing it is probably the better approach until we get the time to really test it.

        I love what you have here, and hope everyone can do that.  It would have been great if we could have.  On the plus side we have a LOT less custom code.

        Oh and I would consider us small to medium – not a large company.

  • Hi Stefan,

    thank you for you detailed description of this process. I think, whenever you get the chance, the money and the time from your business to do such a project it should be done.

    However this is rare. I usually try a more lightweight aproach, so whenever I need to enhance a feature or have to add a feature to this kind of spaghetti monster, I will implement this feature using unit tests in its own classes. And than when I am confident about it’s function I will call it from within the monster. (sure this is not always as smooth).

    And also If possible I will try to fix a feature individiually by moving it into an extra class structure and then just call it again from the old code.

    So the old code gets cleaned up step by step, feature by feature. It takes its time, but I dont need extra budget.

    And also one of my biggest problems is, I don’t know what this old code should be doing. It is like 80% is 20 years old and nobody knows why it has been written and even if it is correct! So it turns out to rewrite that feature when new requirements are clear is a good way too.

    For me real refactoring is only possible if the original code is understandable enough, and I know what it does. If not I tend to rewrite things.

    Cheers,
    Peter