Skip to Content

Are Comments overrated?

Comments are a wonderful tool to document what you intended with certain program logic. You can place it directly where that logic resides: In the source code. As comments are non-functional, they are not getting compiled and hence, not executed. However, I’ve got the feeling that sometimes we should really think about how to use comments and how not to use them. These points are neither fundamentally new nor innovative to developers at all, but I still have the feeling, that these are the most commonly violated principles.

Antipattern #1 – Comments do not replace program structure

Usually, developers should tell the reader of their code why they are doing something, not what they are doing. Before you start to comment by describing what the next few lines of code should do, stop and think about it.

Consider the following example. What do you think would be better to read?


*   parse header from excel
    lv_row = 2.
    lv_column = zcl_excel_common=>convert_column2int( lv_range_start_column ).
    lv_current_column = 0.
    WHILE lv_column <= lv_highest_column.
      lv_current_column = lv_current_column + 1.
      READ TABLE mt_fcat INTO ls_fcat INDEX lv_current_column.
      CHECK sy-subrc = 0.
      lv_col_str = zcl_excel_common=>convert_column2alpha( lv_column ).
      io_excel_worksheet->get_cell(
        EXPORTING
          ip_column = lv_col_str
          ip_row    = lv_row
        IMPORTING
          ep_value = lv_value
      ).
      ASSIGN COMPONENT ls_fcat-fieldname OF STRUCTURE ls_scale TO <lv_value>.
      CHECK sy-subrc = 0.
      CASE ls_fcat-inttype.
        WHEN 'D'.
          <lv_value> = zcl_excel_common=>excel_string_to_date( lv_value ).
        WHEN OTHERS.
          <lv_value> = lv_value.
      ENDCASE.
      lv_column = lv_column + 1.
    ENDWHILE.
    lv_column = zcl_excel_common=>convert_column2int( lv_range_start_column ).
    lv_row = lv_row + 1.
*             …Some more coding…
    lv_highest_row = io_excel_worksheet->get_highest_row( ).
*   parse items from excel
    WHILE lv_row <= lv_highest_row.
      lv_column = 2.
      lv_col_str = zcl_excel_common=>convert_column2alpha( lv_column ).
      io_excel_worksheet->get_cell(
        EXPORTING
          ip_column = lv_col_str
          ip_row    = lv_row
        IMPORTING
          ep_value = lv_value
      ).
      lv_catyp = lv_value.
      lv_column = 1.
      lv_col_str = zcl_excel_common=>convert_column2alpha( lv_column ).
      io_excel_worksheet->get_cell(
        EXPORTING
          ip_column = lv_col_str
          ip_row    = lv_row
        IMPORTING
          ep_value = lv_value
      ).
*                           some more coding…
      lv_row = lv_row + 1.
    ENDWHILE.

Basically, the comments tell me what the next few lines of code will do. But there is no benefit. You just don’t understand immediately, what the code does, no matter if there are comments available or not.

Even worse, more comments would not make clear what the code does. Furthermore, as you shift code to another part of your program, the comments might lose their relationship to the code – as they are non-functional and not taken into account by the compiler. This will eventually lead to comments that are misleading users in the best case.

Consider this refactored example:


    parse_header_from_excel( io_excel_worksheet = io_excel_worksheet io_scale = ro_scale ).
    parse_items_from_excel( io_excel_worksheet = io_excel_worksheet io_scale = ro_scale ).

Actually, replacing comments by special methods with a useful name does not reduce complexity – but it increases readability dramatically.

By the  way, that’s why I think that code scanners which measure the ratio of comments and source code are useless – as the goals that they proclaim to address cannot be addressed by such simple measurements.

Normally, I try to produce code which has no comments in the middle of some logical program unit like a method. If there are comments, they are at the top of the code – or nowhere.

Antipattern #2 – Commented code

Having code that is getting useless by the time will lead to the end of its lifecycle. Usually most of the developers just comment this piece of code out. How often have they ever read this program logic again?

I personally do not read it as it might lose track of its relationship to the surrounding source code by the time. My eyes just went over these comments. Whenever I need to delete some coding that I might miss in the future, I generate a new version of the ABAP source code and delete the code. Code Versioning systems are meant for this kind of tasks. When I miss something in the code, I take a look at the versions – This immediately allows me to compare two versions by the changes that have been applied in the meantime.

To report this post you need to login first.

9 Comments

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

  1. Suhas Saha

    Whenever I need to delete some coding that I might miss in the future, I generate a new version of the ABAP source code and delete the code.

    That’s how it should be done ➕

    And i don’t like the idea of keeping dead code as well. But most of the projects i’ve worked on have this weird rule of keeping the dead code for god-knows-why 🙁

    BR,

    Suhas

    (0) 
      1. Suhas Saha

        Rainer Hübenthal wrote:

        Another way of doing this is just to generat a new Version.

        That’s what i’m saying – generate new version & delete the dead code 🙂

        (0) 
  2. Rainer Hübenthal

    That reminds of a guy who was really an expert in assembler for the mainframe. As i questioned him whya he didn’t commented his code and its hard to maintain the source for others, he promised to do so,

    Some weeks later he presented his new program very proudly. Instead of

    ld a,3

    the source code now was

    ld a,3; load register a with 3

    not mentioning what he intends to do nor what the magic constant 3 would mean in this context 😥

    Any yes, he really commented EVERY line of his assembler program like this.

    (0) 
  3. Nick Young

    Great blog Uwe, perfectly summed up for me in the one line;

    developers should tell the reader of their code why they are doing something, not what they are doing

    (0) 
    1. Uwe Kunath Post author

      You are right, we cover quite the same topics in our posts. Thank you for linking it here. Having two independently written blog posts tells me that this issue is widely existent among developers, so the jury is still out.

      (0) 

Leave a Reply