Skip to Content
Author's profile photo Uwe Kunath

No comment!

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.

Assigned tags

      9 Comments
      You must be Logged on to comment or reply to a post.
      Author's profile photo Former Member
      Former Member

      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

      Author's profile photo Rainer Hübenthal
      Rainer Hübenthal

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

      Author's profile photo Former Member
      Former Member

      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 🙂

      Author's profile photo Rainer Hübenthal
      Rainer Hübenthal

      Oops sorry misunderstood you. Thought you were just copying the report to another one...

      Author's profile photo Jacques Nomssi
      Jacques Nomssi

      I follow Fowler's classification of comments as code smells, but I would not label them antipatterns (cf. dead code antipattern).

      JNN

      Author's profile photo Rainer Hübenthal
      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.

      Author's profile photo Former Member
      Former Member

      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

      Author's profile photo Jelena Perfiljeva
      Jelena Perfiljeva

      There was a similar blog posted on SCN by Martin Voros. Thou shalt search before posting, er? 🙂

      Author's profile photo Uwe Kunath
      Uwe Kunath
      Blog 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.