Skip to Content
Technical Articles

Some thoughts about commenting ABAP code

Do your write code only for machines, or also for your fellow developers and curious functional consultants?

I believe that readability of program code is an important long-term aspect. I try to have it as a big priority and I’m constantly trying to improve my ability to use naming and composition to make the code as self-explanatory as possible. Comments still have their place though, but should not be what the code is based upon.

The online Clean ABAP style guides has a good section on comments, as does the new Clean ABAP book.

I recommend both those sources with all my heart, but let me elaborate here as well how we can write code for humans and machines at the same time…

Four quadrants of code and comments

So if we take for one aspect the naming of things in our code, like variables, classes, methods, etc. We define a very simple 2 grade scale grading how well the naming explains the thing, from ‘Nope’ to ‘Very much so’.

For another aspect, we take the degree to which comments are used to explain the coding. Again with the same 2 grade scale.

Let’s put this in four quadrants for a simplified world.

Four%20quadrants%20of%20code%20and%20comments

Four quadrants of code and comments

Let’s get to it then…

 

1. Nope – Nope

First example is really really common – no comments and no good naming. Probably work ok right when you type it but even after a week, you yourself might not have a clue what your were up to:

lr_tma = go_asmnt->todo( ).
ln_pt = 2.
lr_tma->find_impnt( ln_pt ).
WHILE NOT lr_tma->complete( ln_pt ).
  go_asmnt->fn_main( lr_tma ).
ENDWHILE.

Hey, I made this up myself and I’m not sure I get it… 😀

 

2. Very much so – Nope

All right, so clearly we need to improve on that one… Lets add comments to explain the code:

" Get pending tasks for assignment
lr_tma = go_asmnt->todo( ).

" Sort the tasks in order of priority (sort type = 2)
ln_pt = 2.
lr_tma->find_impnt( ln_pt ).

" Execute all of the tasks, one at a time, in order
WHILE NOT lr_tma->complete( ln_pt ).
  " Execute
  go_asmnt->fn_main( lr_tma ).
ENDWHILE.

Now, finding this code 6 months later, at least we know where to look for maintenance and what it’s supposed to do… It’s much better…

But are we there yet? No, there are still better ways in my subjective opinion…

 

3. Very much so – Very much so

Now, if we instead fix it up a bit and rename our variables, objects and methods…

" Get pending tasks for assignments
tasks = assignments->get_pending_tasks( ).

" Sort the tasks in order of priority
tasks->sort_by_priority( ).

" Execute all of the tasks, one at a time, in order
WHILE NOT tasks->is_empty( ).
  " Execute
  assignments->execute_next( tasks ).
ENDWHILE.

Now the naming is explaining the code and the comments are explaining the code. Ok, all good? Twice as good? Well. Yes but no. Since the coding basically says the same thing as the comments now, we need to read the same thing twice… And it also has to be written twice… And changed twice when making adjustments or adding features…

And what if the comment and the code is not saying the same thing? Do we trust the comment or the code?

Let’s carry on to the final one and my personal favorite…

 

4. Nope – Very much so

tasks = assignments->get_pending_tasks( ).
tasks->sort_by_priority( ).

WHILE NOT tasks->is_empty( ).
  assignments->execute_next( tasks ).
ENDWHILE.

Ah! The cleanliness. Quick to read. Quick to get an overview. Understandable code.

Just as Jules is trying real hard to be the shepherd in Pulp Fiction, I try real hard to write this kind of code. I’m not where I want to be yet, but I’m trying real hard…

 

5. What, that’s it? Just like that? No comments ever?

Well no. First, there are a couple of important prerequisites for this… One prerequisite is a good composition of the code, the division of code into clearly defined and scoped classes and methods. If your code is 1500 lines in a procedural style, then naming things well certainly help, but you still risk getting lost anyway.

And a prerequisite to do that well is to use ABAP OO, which we all should whenever possible!

Another is context. I mean in the above example there are still many things we can’t say other than that this code is supposed to be prioritizing and executing. Hopefully it’s clear from the context where the code is located what an assignment is and what kind of tasks we’re talking about, for instance.

Comments are still a very good way to explain WHY something is done in a certain way, that might not be obvious… Something like this, for example:

" For some reason, the BAPI_CREATE_SOMETHING is not accepting item_id
" to be '0001' when creating a new object and assigning items at the
" same time. We use '1337' to prepare the data instead. This is fine.
items = VALUE #( ( item_id     = '1337'
                   description = some_data->title
                   value       = some_data->value ) ).

(It’s funny how the styling of the code blocks here are syntax highlighting inside the comments…)

 

Now, if only the reality was this simple… 🙂

Stay safe everybody and let’s hope to understand each others code…

Addition: To keep reading on this topic in the SAP Community, I recommend continuing with I See Dead Code after getting the extra value from the comments below.

/
Four%20quadrants%20of%20code%20and%20comments
32 Comments
You must be Logged on to comment or reply to a post.
  • My favorite subject 🙂 So much can be achieved with little effort if you choose names appropriately.Although choosing the right name sometimes takes a long time. At least for me. But it's worth it.

    I cannot live completly without comments in my source code. I don't want that either. But I know that a comment is really important when I see it 😉

    By the way: I work a lot with the ABAP Development Tools. If you write methods in it, the short text is missing in the SE24 and also in the ADT via F2 (element information). That's why I now use ABAP Doc to give some methods a synchronized english short text. But only short text and it must fit in one line 🙂

  • Very well written. Unfortunately with comment to code relation checks the ‘what is the following code doing’ (or worse method history with change log.. Brrrr…) comment style is enforced. Let’s hope for the day where the last one was convinced that this measure doesn’t make any sense 🙂

    /
    🙂
    • I remember being told I had to comment a bit of code not much more complex than this.

      METHOD read_data.
        READ TABLE some_table WITH TABLE KEY field = i_value.
        IF not_found( ).
          MESSAGE 'Not there' TYPE 'I' DISPLAY LIKE 'E'.
        ELSE.
          MESSAGE 'It is there' TYPE 'I' DISPLAY LIKE 'S'.
        ENDIF:
      ENDMETHOD.
      
      METHOD not_found.
        r_result = xsdboolc( sy-subrc IS NOT initial ).
      ENDMETHOD.

      I asked "what comment would you like me to put in"?

      Change logs -> yes, they became part of the standard in my last year at my previous place of work. I'm glad I left.

    • Can relate. I ran into this requirement with the ATC check in one project: code must have X comments per X lines or something. Honestly, I don't understand why this is necessary and what is the logic behind it. Sometimes short code may need a comment and sometimes longer code is self-explanatory. Whether to comment or not has nothing to do with the code size. (I have yet to see any research that would find otherwise.)

      I wish more SAP customers would look into practicality and "cleanliness" of their ATC checks instead of just blindly adopting something. Otherwise it defeats the purpose of having any quality check.

  • My comment to your code without comment would be "ante up, use an iterator":

    tasks = assignment->new_iterator( ).
    WHILE tasks->has_next( ).
      assignment->execute( tasks->next( ) ).
    ENDWHILE.

    Even in this code, I would like to add a comment asking if assignment is really the right object to be responsible for executing a task. So maybe I would add a dimension "comment explaining the abstraction".

    But I like your quadrants simplification very much.

    regards,

    JNN

    • Yes, Jacques, you have a perfect case-in-point here, thank you! Context is really necessary for understanding the code. Depending of what assignment is and what tasks are, your implementation with the iterator pattern might be the perfect solution. And assignment and tasks may be totally wrong names. 🙂

       

  • Interesting discussion, thanks for starting it Jörgen!

    I like to put comments into code (and may be overdoing it sometimes!), mostly to explain why something is done. Here are two examples from a program I recently wrote utilizing the RTTS-framework for the first time.

    and

    Without comments like these, I wouldn't remember for long what and why I did there!

    Note: I had copied the RTTS-logic from one of the DEMO-programs which used prefixes and form-routines and as that already required some "wrapping my head around" about what's going on there, I didn't dare try to convert this to ABAP-OO and mess with the names used. And, I wasn't even sure if ABAP-OO would have been an option for RTTS given the dynamic nature of the structures used.

    Cheers

    Bärbel

    • Thank you for the real world actual examples.

      I totally agree with the why-comments... And I really like you also adding the links in there for more information, to give that extra added value.

      I had only two axis in the simplified worldview I described, but I guess another one could be "complexity" or something. Like "Is this understood by the majority of ABAPers or do I need to explain this command or what I'm doing here?". For instance, many developer are not used to the VALUE operator yet, but I think that one is pretty straight forward. Just a F1 away. But I guess it can be easy to get carried away and writing an overly complex statement using the REDUCE operator...

       

    • With your example here, (notwithstanding your caveats), I would have modularised like this:

      IF p_compr EQ abap_true.
        do_stuff_with_meaningful_name_1.
      ENDIF.
      
      " do_stuff_with_meaningful_name_1
      READ TABLE...
      IF sy-subrc EQ 0.
        do_stuff_with_meaningful_name_2.
      ENDIF.
      
      " do_stuff_with_meaningful_name_2
      DATA...
      WHILE...
        do_stuff_with_meaningful_name_3.
        exit loop if done.
      ENDWHILE.
      
      " do_stuff_with_meaningful_name_3
        role_and_user...

      And then adjusted the WHY comments accordingly.

  • I see a comment before a block of code as an indicator that perhaps the code should be in its own method. I generally try to do that whenever it turns up. It’s no always quite so easy, so with established code, sometimes I just have to leave it.

    I do put comment lines above SE91 messages used in code with the text. It's handy to have some indication. Of course that leaves the possibility that the message will change and the code won't... 

     

     

    • There is a specific rule against that in the clean code guidelines, but I agree with you on this one (as well). It's true that the message details and phrasing might change. But I consider those as explaining the why rather than indicating the exact error message text. "Ok, so we're issuing a MESSAGE here, why is that?". It's not always obvious for instance if no match from reading a table is totally fine or if it's really really bad.

      And if messages are repurposed to mean a totally different thing later on, well, then there are bigger problems... 🙂

  • This seems like a nice opportunity to pimp my not so old blog I See Dead Code. The "Begin of changes... end of changes" fall somewhere near 0 in your quadrant. 🙂

    "Narration" of the code is my second pet peeve ("*Here I'm selecting data from database" followed by SELECT). This argument: "the comment and the code is not saying the same thing" should, hopefully, hit the last nail in its coffin.

    • "Program is a living organism and dead code is as useful as a gangrene. Let it rest in peace in the version control."

      Love it!

      I'll actually go ahead and add a link to your post above, in case somebody misses these comments... But I hope nobody does, as there is great extra value in the comments both here and in your post. Thanks!

  • Maybe I would use this:

     

    try.

    assignments->execute_pending_tasks( im_sort = by_priority ).

    catch zcx_execution_exc.

    endtry.

     

    If the code is well organized and named then comments can become easily just a duplicate of code and then I would rather omit them. Sometimes it is not easy to create such a code and some help from comments is needed.  One problem with SAP I see is that I have only 30 chars long names, which is short if we take into account that we need to use some prefix also (like ZCL_ ...here 4 chars are lost...and there are even teams which use much longer prefixes because of their local dev standards and it can happen that you have only 20 chars remaining for some meaningful name). One reason why it is difficult to create meaningful names is that this require much of thinking to create good design. It is close to impossible to find a meaningful name if ta code design is low quality.

    • Hah! Yes, it doesn't take many characters for a regular expression to be hard to understand.

      Agreed on the necessity to comment that one if it's not encapsulated in a super well named single method, which will probably never be the case.

      Out of curiosity, do you tend do document only what it does, or also the explanation like

      " ^(\d*)HELLO
      " ^            - This is the start of the string
      " (\d*)        - First subgroup
      "  \d          - digits
      "    *         - any number of, in this case, digits
      "       HELLO   - the literal string 'HELLO'

      I mean I see the benefit of the explanation but for a longer string, and if in need of maintenance, how about this? 😀

      " Just paste this regular expression into regex101.com.
      " You will get the explanation and have the possibility to test it
  • Oh I liked the four quadrant approach. Never thought about it this way and it's so easy to visualize and use it. I think much of this variable naming and commenting discipline starting with when we start learning the programming language. I remember the times in my QA reviews when naming a variable as l_count meant that we are counting something and then invariably getting into l_count1, l_count2 and so on often forgetting what that count was meant for. Same for l_flag. I also think that on occasions where our only requirement is do a bug fix in some incident management engagement and our scope is maybe writing 10-15 lines of code, the programmer should make an attempt to write them in as clean manner as possible ignoring how the remaining program was written.

    Couldn't agree more on the "WHY something is written" instead of the narrative.

    Thanks for sharing your thoughts

  • Fully agree about choosing a meaningful class/method name, however the 30 character limit (minus prefixes and underscores) is usually an issue.

     

    On the side note, I still see lots of "comments" like this:

    * Sorting the internal table
    SORT itab.