Skip to Content

When looking at older ABAP programs, which I quite often do for analysis and maintenance purposes, it often strikes me that a large number of code lines have been ‘commented out’: an asterix (*) has been placed before the original code. Every ABAP-programmer knows code like the (fictitious) example below: old code has been replace by new lines to improve the code or change its functionality.

* BEGIN OF DELETION BME20130101
*SELECT * FROM vbak INTO order_header WHERE vbeln IN so_vbeln.
*  SELECT * FROM vbap INTO order_line WHERE vbeln = order_header-vbeln.
*    “here, some fancy stuff happens.
*  ENDSELECT.
*ENDSELECT.
* END OF DELETION
* BEGIN OF INSERTION BME20130101
SELECT * FROM vbak INTO TABLE order_headers WHERE vbeln IN so_vbeln.
IF lines( order_headers ) > 0.
  
SELECT * FROM vbap INTO TABLE order_lines FOR ALL ENTRIES IN order_headers
    
WHERE vbeln = order_headersvbeln.
  
LOOP AT order_headers INTO order_header.
    
LOOP AT order_lines INTO order_line WHERE vbeln = order_headervbeln.
      
“here, some fancy stuff happens.
    
ENDLOOP.
  
ENDLOOP.
ENDIF.

* END OF INSERTION


This practice is often advocated or even made obligatory in development guidelines. When asked why, I often get one or more of the following reasons:

  • To improve maintainability of the code. Using this practice, other programmers are better able to track changes to code and thus analyze possible errors.
  • It’s easier to revert to older code versions.
  • I still remember what I was doing the next day and If I get sick, other people can see what I was doing.
  • It was a practice in my previous job as an ….programmer where we didn’t have any source control tools.

I consider commenting out ABAP code (or for that matter, any code) a bad practice because 1) it affects readability and thus maintainability in a negative way, and 2) you don’t need it because you can use version management:

  1. The code gets clogged by old code that is not relevant anymore, which seriously affects the readability and thus the maintainability of the ABAP code. As someone puts it “sometimes it’s like excavating a tomb to find the actual code that is still being used. (http://stackoverflow.com/questions/123423/do-you-leave-historical-code-commented-out-in-classes-that-you-update)
  2. SAP offers Version Management for most workbench objects. (why SMARTforms has been excluded from version management still beats me 🙁 .) It is possible to compare versions on the same system or on another connected SAP system. This way, you can always check the version that is running in production. It is also possible to revert to an older version or to store an intermediate version. To be sure, version management in SE80 could be improved by offering better tools to merge different versions, but I expect that ABAP in Eclipse offers much better options for that. I still have to check that out.

Commenting out code only makes sense sometimes during developments, when your not exactly sure if your changes are correct. Then these comments can help you to quickly understand what is happening, why your changes don’t work as expected etc. However, when your code is fully tested and ready for transporting it into production, take a few minutes extra to delete all the code that was commented out. And while doing that, maybe it’s also a good idea to delete unused variables and even routines or methods.

Should the code have no information at all about the changes that were implemented? No, that’s not what I think. In my opinion, adding a short, to-the-point description of an implemented change in the header of a program, a function pool, a class pool, preferably with a link to issue tracking identification and other types of documentation, is worth considering, because it helps you and subsequent programmers to understand the current functionality and technical design of your code. Just make sure that this inline documentation, just as all the other documentation is relevant, up-to-date and correct. But that is a subject for another blog.


To report this post you need to login first.

18 Comments

You must be Logged on to comment or reply to a post.

  1. Simon Rotherham

    A well argued case.  I’m largely in agreement with you.  The ABAP version management functionality is excellent and tells the true picture of the change history.

    I do find it irritating when you come across a program that has more commented out lines of code than active ones.

    Most requests on developers are about changing or enhancing the current code set not identifying who changed what and when.  And for the latter, the version management tool is by far the better option.

    (0) 
  2. Paul Hardy

    Whenever I am maintaining a program and find these commented out lines I delete them. Every time.

    That’s what the version management is for, as you said.

    In one of my blogs I talked abot a situation I encountered where a routine was millions of lines long, with 90% of it commented out, with little islands of live code, feeling really lonely, wondering whre all their friends had gone.

    How can you understand what something does if you have to page down ten times to get to the next bit of code – by the time you have reached the next bit you have forgotten what the last bit did!

    I am in favour of adding comments such as “please don’t change this code back to doing such and such, we tried this once, it did not work and here is why” otherwise you may get into an endless loop….

    (0) 
  3. Craig S

    OK.. too many folks in favor here of getting rid of these comment lines!!  I just have to play devils advocate.  It’s what I do!!!  😈

    I can understand that from a programmer’s point of view the comment lines are a pain in the a**.  From a purely practical view I get it.  It makes it hard to read.  If you really have a program that is hard to read, copy it into excel, filter out the lines that start with “*”.  You can read it right there if you are just doing some investigation or research.  If you want to use it as a template for a new program delete out the lines that start with * and save it as a text file, paste it back into SAP and program away.

    When I come into a company with an existing SAP system and I’m asked to look at something, it’s unbelievable how difficult it can be because of a lack of comments.  It can be important to understand why a particular change was made, why a specific check or exclusion was incorporated into a program.  Many of these changes get put in over the years.  Seeing that a particular SAP note was applied helps greatly too.

    The problem is reality.  I’ve seen very few companies in my travels that REALLY have a good version control or good repository of functional specs.  Trying to unravel a conglomeration of functional and technical specs that are supposed to be maintained is often daunting.   Also given that MOST folks give short shrift to the documentation and do the minimum to meet standards means descriptions and reasoning are often lacking in these documents. 

    Sometimes I think the big consulting firms figure they can charge more by making their own specification documents and systems for a client and the bigger and more complicated they are the more they can justify their charges.  Please just give me a simple Word doc that is regularly updated without all the unused sections found in the one document fits all.

    And also  when tweaks, corrections, additional functionality not in the original request, unforeseen issues that need to be handled are all introduced into the program during prototyping, testing and validation, many of these items never find their way back into the original functional spec or technical spec.

    Is this an excuse to request programers to document the hell out of program?  No.  Poor version control, poor change control and update procedures are no excuse.  But it is reality. I don’t really care who changed something or even when.  The why is the most important!

    When you are programming, please keep in mind that your audience for those comments is not necessarily just a future programmer.  It will often be someone like me who knows enough ABAP to get in trouble but needs to usually look at these various objects and try to figure out what went before him/her.  Just getting a clue as to why the code excluded a particular movement type or document category can save hours of research and headaches.  Your comments can be a lifesaver years down the road when we try to figure out what the heck a business did and why they did it after the original folks involved are scattered to the four winds!

    Anyone whose tried to unravel COA programs and all the related exits, FM’s and SAPscript for an upgrade knows exactly what I’m talking about. 

    FF

    (0) 
    1. B. Meijs Post author

      Hello Fire Fighter,

      Thanks for taking time to write your reply. I’m not saying that comments should be deleted from the code, but commented out ABAP code lines. As you rightly say, inline comments in an abap program are very important to understand why decisions were made because offline documentation is more often than not not up-to-date, not correct or simply not available. If you replace code and its important to document the decision why you did that, add that reason as inline documentation, maybe even at the place where you changed it. But delete the original code. If you really need to know what the code looked like, just check version management.

      Regards,

      Ben Meijs

      (0) 
      1. Craig S

        Yeah.. we might have to agree to disagree here.. 🙂

        I don’t think there is any one specific answer for this.  Maybe certain parts of the code might be able to be deleted without losing context.  But I think other parts should be left intact. 

        For instance, a common inline document line might be “Checking for valid order types”

        If you remove the related code, we might have no idea what order types where checked or why years later.

        Lets say you had logic in there to run another Z program when it was order type ORD1.

        Now a few years later we add ORD2 to the check and delete the ORD1 check.

        Now we come back a year later and some one says we want to put ORD1 back in.  Would we know that it was in there at one time? Is it important?  I don’t know.  You’d never know at the time of coding.  Maybe you might mention the change in the inline documentation… probably not… in most places it would still say “Checking for valid order types”.

        I’m not saying you have to document the business cases and decisions in there in huge detail but having the old code can help down the road to help understand what happened and why.

        FF

        (0) 
    2. Otto Gold

      Hello.

      I see where you’re coming from. But you can’t try to fix the mess by keeping old code there as a replacement for meaningful comments. I can understand the reality is sub-optimal, but that is a organizational problem, not a technical one. There are tools (like ATC, SolMan etc.) that help with code governance and that is the way to go.

      See you around, I am a fan of you BTW 🙂

      cheers Otto

      (0) 
      1. Craig S

        Thanks Otto..  The admiration is mutual I assure you.  🙂

        I guess the question is one of balance within the code.  Making sure it’s clear of the intent and what the code is performing. 

        One problem I have with some tools is that they change at the whim of every new IT manager. Almost as much as programming languages change.

        I have had several projects I’ve gone in on and I get “well.. all our previous documentation was in Lotus Notes and we’ve mothballed that now.  If you know exactly what you want I can probably get a copy but it’ll probably take a week or so”

        Or “We had all that on a server, but we no longer use that and we archived everything…. “

        As you said.. all organizational problems of course.  IT changes so fast and often that I no longer trust that whatever tool is used today, will be available or relevant a year from now.  The lowest common denominator is still the code itself.

        Craig

        (0) 
  4. Naimesh Patel

    Hello Ben,

    I also have to disagree a bit here. Most of the points raised by Craig S (which I believe is not an actual name 😉 ) are valid and I have to agree with it.

    My two cents:

    Agree that there is a good version management for most of the objects, but for few instances when you have a critical incident – Like replaced a SELECT in the output driver program, which is issue immediately, would end up not saving the documents at all resulting into Critical incident. When you are in this type of situation where near-zero critical incident is only expected, you would need to revert back the changes. Having the commented code out there with the begin/end of marker would help you understand the change’s purpose and help you to easily fix it.

    You also mentioned:

    However, when your code is fully tested and ready for transporting it into production, take a few minutes extra to delete all the code that was commented out. And while doing that, maybe it’s also a good idea to delete unused variables and even routines or methods.

    Many times, making a change in tested codebase is considered foul-play. In bigger organization where there bigger release management teams, this would be on you if codebase was changed after the test and it not working for any reason in the production. Release management teams would advise to revert back the entire thing resulting into a higher development cost. So, I would never advise anyone to touch tested codebase, of any sort.

    Thanks,
    Naimesh Patel

    (0) 
    1. B. Meijs Post author

      Hi Naimesh,

      thanks for your reply.

      You said:

      When you are in this type of situation where near-zero critical incident is only expected, you would need to revert back the changes. Having the commented code out there with the begin/end of marker would help you understand the change’s purpose and help you to easily fix it.

      You can revert back the changes using the version control. You don’t need the commented out code lines for that. Commenting out code doesn’t really help understanding why something was changed. If you need that information, it should be added as inline comment to the program.

      With regard to deleting non-used variables etc, your wrote:

      Many times, making a change in tested codebase is considered foul-play. In bigger organization where there bigger release management teams, this would be on you if codebase was changed after the test and it not working for any reason in the production. Release management teams would advise to revert back the entire thing resulting into a higher development cost. So, I would never advise anyone to touch tested codebase, of any sort.

                         

      I think you are right in saying that you should not change the code anymore once its fully tested. The timing I proposed for that is indeed not ok. But notwithstanding that, cleaning up code should be considered, but maybe earlier in the change process (after you finished your program tests) and also not during issue solving. My point is that it’s worth investing just a little of your time to clean up programs.

      Regards

      Ben Meijs

      (0) 
      1. Paul Hardy

        In regard to deleting non-used variables and the like…..

        After I have finished with a program I run the extended syntax check and the code inspector. then on a case by case basis I decide whether the automated checks are being hyper-sensitive, and if so se a pragma to suppress the message, or if I feel the suggestion is valid – and 95% of the time they are, I make the suggested change.

        This is what the “official policy of SAP’ whatever that’s worth suggest in books like “Official ABAP programming Guidelines”.

        Am I crazy to do this? I know for a fact that we have had several program changes go into production which stuffed things up violently, and they could have been prevented had the programmer run the extended program check. Do any companies have a non-SLIN policy? Can the real SLIN Shady please stand up?

        In regard to changing “the tested codebase”, here I am going to shamelessly plug my blog http://scn.sap.com/community/abap/blog/2013/04/18/are-you-writing-evil-abap-code

        In essence what you describeas foul play is that if you make ANY change to the program not directly related to the problem at hand then you have the fear of breaking something, and then if it all goes wrong you get sacked / murdered.

        There is a miracle cure for this, and it is the ABAP Unit Test Framework. It is one hell of a lot of work, but once you have got to the top of the mountain you can make any change you want and be 100% sure you have not broken anything.

        There is so much about this on the internet – test driven development / behaviour driven development etc and I found the ABAP versio of this to be a concrete benefit to me about ten minutes after I started going down this path.

        If all this sounds too good to be true, give it a go yourself, and tell me what you think. I haven’t heard anyone slag off unit testing yet, but I wait with baited breath.

        Cheersy Cheers

        Paul

        (0) 
  5. Nigel James

    Great point Ben I agree that

    Commenting out code only makes sense sometimes during developments,

    and you only get the benefits of version management when  a version is released.

    In the rest of the world git and svn allow much more fine grained version control and you get a better picture of what was changed by who and when – not just when a transport was released

    I think with AIE (ADT) you will be able to use these tool much more effectively.

    (0) 
  6. Avirat Patel

    Hi Ben,

    I am strongly agree with the lines…

    Commenting out code only makes sense sometimes during developments, when your not exactly sure if your changes are correct. Then these comments can help you to quickly understand what is happening, why your changes don’t work as expected etc. However, when your code is fully tested and ready for transporting it into production, take a few minutes extra to delete all the code that was commented out. And while doing that, maybe it’s also a good idea to delete unused variables and even routines or methods.

    When you are sure about your final code along with fully tested unit ..i think there is no space for commented code.

    But commenting a code for every line is worthless rather then you have to just mentioned what is in that specified logic[or what this code is going to do]..in just couple of lines.

    – Avirat

    (0) 
  7. Paul Hardy

    Here is a quote from Robert C Martin from his book “Clean Code”:-

    “it makes me crazy to see stretches of code that are commented out … that code sits there and rots, getting less and less relevant with each passing day …. commented out code is an ABOMINATION … when you see commented out code DELETE IT!

    Hmmm, not much gray area there.

    (0) 
  8. James Wood

    Well said. This practice has always been a pet peeve of mine. The ABAP Workbench has great version management support so why not use it? The same thing goes for flower boxes for me. I prefer to keep the code lean so that we can jump right in and see what’s going on.

    (0) 
  9. Otto Gold

    Hello,

    what I do is that I put a date of change into the source code when I change the code (and leave the old version there) and the next time I touch the code I remove the dead wood. This way I can quickly see what changed but don’t leave dead wood behind. Leaving dead code behind forever is IMO careless and a very bad practice.

    cheers Otto

    (0) 

Leave a Reply