Skip to Content
author's profile photo Jelena Perfiljeva

Share your "ABAP gore" Part 2

By popular demand, this is a sequel to the original post, which grew unwieldy with 300+ comments taking long time to load. Same purpose as the original post:

"There is a sub on Reddit called Techsupport gore where people share pictures and stories of some horrific things they see done with technology.

I've been looking at some old programs lately and see lots of stuff that would qualify as "ABAP gore". :) So I thought it might be therapeutic and educational to post some examples on SCN (examples to follow in the comments)."

* Please Login or Register to Comment on or Follow discussions.

57 Comments

  • Feb 18 at 09:21 PM

    I got an email from the users asking to explain what triggers an error message they were getting. Thankfully, it wasn't one of those "& & & &" messages but, even so, it occurred in about 10 different programs. Below is just a part of what I found (relevant definitions provided for context, comments mine, some variable names changed to protect the innocent). As a side note, more or less the same code repeats in all the different programs, not a single time an effort was made to create at least an FM.

    DATA: valid_char(36) TYPE c   " Not a constant
     VALUE 'ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789'.
    DATA: some_variable TYPE c LENGTH 10. FIELD-SYMBOLS: <valchar>. len = strlen( some_variable ).
    ASSIGN some_variable+0(len) TO <valchar>.
    IF <valchar> CO valid_char.
    ELSE.
    " display vague error message
    ENDIF.
    • Feb 19 at 06:25 AM

      ( Jelena Perfiljeva thanks for this new place !! )

      There are a lot of points on these lines .... SY-ABCDE / ASSIGN not protected ... and REGEX that do the job in one line.

      Maybe, what could be nice, it is a Blog/Wiki where we could propose simplification of strange code found ?

      • Feb 19 at 06:31 PM

        Great point, Frederic Girod ! As stated, the purpose of this discussion is not merely to poke fun at the code but also educational. After all, a lot can be learned from how NOT to do ABAP.

  • Mar 03 at 10:39 PM

    I am happy to see that when something is zero then the "not zero" flag is set. The other day I also saw a variable called COSTED which was set to X when the order had not been costed.

    not-zero.png (9.2 kB)
    • Mar 05 at 04:12 PM

      Hahaha! This reminds me of a rather cheesy standup comedy trick of saying something and then immediately exclaiming "Not!" :) "Order costed. Not!"

      Extra points for using 'X' too. :)

    • Mar 31 at 06:19 AM

      Oh, I can explain: surely the code was written on Opposite day! (German: "Gegenteiltag").

      Wow, it (the day, not the code!) even has an article on Wikipedia!?

  • Mar 05 at 12:54 PM

    This one gave me a laugh "unless on is gung-ho"

    start-of-selection.
    
      " this report reports the truth ( raw persistence, unless one is gung-ho ...
      set parameter id '/UI2/PAGE_CACHE_OFF' field p_caoff.
    
      data lr_packages   type ref to /ui2/if_fcc_const=>TP_T_PACKAGES.
      data lv_scope      type          string.
    
      data l_i_signature type abap_bool value 'X'.
      if ( g_signat = 'X' ).
        clear l_i_signature.
      endif.
    • Mar 05 at 04:13 PM

      "this report reports the truth" - haha, I'll have to remember this one, maybe will use it in a title one day. :)

    • Mar 09 at 07:08 AM

      I wonder how these lines of code could be written ? Is it a a result of several people modifying without understanding the first objective ... ?

      • Mar 12 at 12:15 PM

        Perhaps. The comment itself seemed strange to me. The lines of code are strange, as this is the start of the program. But I didn't really dig much deeper to try to find out why they did this at the start of the program. But I'm not sure where g_signat was found

        data l_i_signature type abap_bool value'X'.if( g_signat ='X').clear l_i_signature.endif.
        • Jun 18 at 09:59 AM

          Assuming g_signat is a global flag of somekind, what is the added value of a local flag pertaining to it?

          This is hurting my brain. The only way you would use redundancy in coding for such a simple variable is maybe for readability during debugging where the same value has a different functional meaning elsewhere.

  • Mar 12 at 12:38 PM

    Before reading the code, the functional guy said "This code is special, but nothing is done without reason"

               read table t_dliv with key kunnr = t_paret-kunwe.
               clear w_index2.
               loop at T_MDVEX.
                   w_index2 = w_index2 + 1.
                   loop at t_dliv where kunnr = t_paret-kunwe.
    • Mar 12 at 12:47 PM

      same code ...

                     loop at t_dliv where kunnr = t_paret-kunwe.
                       if t_dliv-zdatu GE T_MDVEX-DAT01.   
                          exit.
                       else.
                          clear t_dliv-zdatu.
                       endif.
                     endloop.

      Why CLEAR ??

  • Mar 16 at 05:40 PM

    Class /IWBEP/CL_MGW_ABS_DATA . Yes one can comment too much.

      CONSTANTS:
      BEGIN OF gcs_association_multiplicity,
                 one         TYPE string VALUE '1',    " Indicates that exactly one entity type instance exists at the association end
                 zero_or_one TYPE string VALUE '0..1', " Indicates that zero or one entity type instances exist at the association end
                 many        TYPE string VALUE '*',    " Indicates that zero, one, or more entity type instances exist at the association end.
               END OF gcs_association_multiplicity .
    
      FIELD-SYMBOLS: <ls_deep_component> TYPE any.
    
    *-get service namespace (basically service name) from application context object to fulfill API contract
      mo_context->get_parameter(
        EXPORTING iv_name  = /iwbep/if_mgw_context=>gc_param_service_namespace
        IMPORTING ev_value = lv_ctx_service_namespace ).
    
    *-get child nodes of current expand node
      io_expand_node->get_children( IMPORTING et_children = lt_children ).
    
    *-check whether current node has more children
      IF lt_children IS NOT INITIAL.
    
    ...and on, and on, and on...
    • Mar 16 at 06:45 PM

      We have some programs like that at work. :( This type of "code narration" style is driving me crazy. For cripes sake, let the code speak for itself!

      • Mar 16 at 10:22 PM

        Seen plenty of it in customer code, but this is recent-ish SAP code - it's deep in the core of the Netweaver Gateway / OData service.

        And if someone reading this doesn't know what 0..1 multiplicity is then they need far more help than the comment...

  • Mar 17 at 12:37 PM

    Not an Abap Gore, but a funny Standard SAP Code in the ATP Check

    *   these are the adventures of the new spaceship BERID;
    *   she intrudes into MRP areas where no man has gone before;
        PERFORM DISPO_AREA_SET TABLES P_ATPCSX
                                      P_ATPMATX.
  • Mar 23 at 01:31 PM
    **************************************************************
    *** There is no point in doing a RETURN here since we are done here
    *** anyway.

    Or... could have just put RETURN. Or nothing. Instead of a three line comment that adds no value whatsoever.

    • Mar 23 at 06:31 PM

      Sometimes I feel there should be a section of Psychology dedicated to the program comments. Could be an interesting research field.

  • Mar 30 at 06:07 AM

    Have a look at the buffering settings on standard table T001W.

    Can you spot what is wrong?

    • Mar 31 at 06:28 AM

      Actually, I can't!? Seem's ok to me....
      Care to explain?

      (Screenshot from a recent system - S/4HANA 1909 , in case that matters )

      • Mar 31 at 06:39 AM

        This is something that has caught us out a few times in my company. People get really confused as to whether MANDT counts in the number of key fields. Clearly it does.

        So in this case T001W is generically buffered on MANDT. As there are two key fields that is the only way you can possibly generically buffer it. This means the first time someone does a read on T001W in a client the entire table is loaded into the buffer. It is, in effect, fully buffered.

        Take TVKOT as another example. That is also generically buffered on MANDT. I was always told that for text tables you should generically buffer on SPRAS. In our Australian system we only have one country as my German colleagues have pointed out once or twice, so we only ever log on in English. Yet the standard German values in TVKOT get loaded in as as well.

        I put it to you this was never intended. The person who created the setting on TVKOT thought MANDT did not count and so set the number of fields to "1" trying to buffer on SPRAS.

  • Apr 01 at 03:03 AM

    There are lots of things wrong with this, but the one that makes my head spin is that by passing in "E" to the LR_EXCLUDE range the last instruction is to delete everything that is NOT in the exclusion table.

  • Apr 01 at 10:48 PM

    If typing out "CHAR1" for a single character data type is too much work, just use this handy type definition:

    class /IWBEP/CL_MGW_PUSH_ABS_DATA ...
    ...
    public section.
      types S type CHAR1 .
  • Apr 03 at 05:34 AM

    Include LBTCHFXX contains this form.

    FORM gen_jobcount USING jobname jobcount rc.
      DATA: jcnt LIKE tbtco-jobcount.
    ** FIXME
    ** Diese sehr rudimentaere Methode soll durch Nummernkreisvergabe
    ** ersetzt werden
      GET TIME.
      jcnt = sy-uzeit.
      rc = 1.
      DO 99 TIMES.
        IF sy-index > 9.
          jcnt+6(2) = sy-index.
        ELSE.
          jcnt+6(1) = '0'.
          jcnt+7(1) = sy-index.
        ENDIF.
        SELECT SINGLE * FROM tbtco
               WHERE jobname  = jobname
                 AND jobcount = jcnt.
        IF sy-subrc > 0.                   " freie Nummer gefunden
          jobcount = jcnt.
          rc = 0.
          EXIT.
        ENDIF.
      ENDDO.
    ...
    ENDFORM.

    For those who don't read German, that comment means

    "This very rudimentary method is to be replaced by number range assignment"

    This comment was put in between 4th July 1996 and 15th July, 2013. We're still waiting somewhere between 24 and 7 years later! (My money is on the 24 years).

    • Apr 03 at 12:05 PM

      I'll join your bet.

      I can confirm it's not later than November 2008.

    • Apr 03 at 06:58 PM

      "There is nothing more permanent than a temporary fix" (c)

      I bet some code I wrote back in 2005 as a "temporary workaround" still runs in Production. [blushing]

    • Apr 04 at 07:57 AM

      Forget the comment, I love the roundabout keep-stabbing-at-the-database-until-we-find-a-hole approach.

  • Apr 06 at 07:47 PM

    Custom (Z...) table has a key field named EBELN (= same name as the key in standard PO tables EKKO/EKPO). However, the field is assigned a custom data type that is using CHAR10 domain. Even though the length is the same as standard EBELN data type, this one doesn't use ALPHA conversion routine.

    Now I have a fun task of ELI5 to the users how '100' is different from '0000000100'.

  • Apr 12 at 02:53 AM

    I just decided to change the name of a local variable I came across. The previous name was LD_VALUE and for some reason I did not think that was descriptive enough.

    • Apr 14 at 09:29 PM

      I do find the use of LD_ wrong. D is for Data, which is the most generic type in the ABAP universe, it includes tables and structures, yet LD_ is used denote an elementary data type.

      Another reason to drop prefixes...

  • Apr 12 at 09:31 AM

    Now I came across the conditional logic that followed the above comment today.

    "This condition will be removed"

    Do you know, when someone writes a comment like that, I honestly think they actually believe what they are saying.

    • Apr 12 at 11:40 AM

      You should have waited for year 2012 at least...

    • Apr 13 at 04:05 PM

      Well, when you have 9986 lines things slip up eventually. :)

      • Apr 13 at 10:53 PM

        There are five INCLUDES each one with way more lines than that. For 20 years code has been added non-stop with nothing ever removed when it was not relevant any more.

        I am putting any new logic in an "Island of Happiness" using TDD. I am giving a presentation on what I am doing at SAP Online Track on 30/31 May, plug, plug.

        • Apr 15 at 07:43 PM

          "nothing ever removed when it was not relevant any more" - this is my pet peeve in the legacy code. 20 years, no one stops to run Extended Check (which existed for a long time already) or to think how to improve even a little thing. And it just gets worse and worse.

  • Apr 23 at 02:58 PM

    Just. In. Case. It. Is. Not. Clear.

  • Apr 23 at 08:10 PM

    As a nod to this somehow continued discussion about Hungarian Notation, here is just a taste of what I found today in one program:

    t_prot - global table

    f_stat - global variable

    p_vbeln - USING parameter for a routine

    p_remove - CHANGING parameter for a routine

    Local variables: lv_menge, xdocnum, o_vbeln (no, not an object).

  • May 04 at 09:49 PM

    Nice collection of random global variables (and yes, there are 300+ lines of them). Can't make this up. If I was looking for an example of bad naming choices and strange definitions this would be it. Feel free to use this image for educational purposes, free of charge.

    scn.jpg (95.1 kB)
  • May 13 at 11:28 AM

    OK I found a user exit which was evaluating system field SYST-ONCOM. I had never even heard of that. Anyway if the value was "T" something special happened.

    Weeks later I found (by accident) what was setting that field to "T".

    So someone had decided the best way to transfer data from one place to another was to pick a random field from the SYST structure which they did not know what it did, fill it with a random value, and the hopefully that system field will not have changed by the time the user exit evaluated it.

    oncom.png (6.3 kB)
  • May 13 at 03:37 PM

    A piece of code from the fifth of BIG4

    Very clean boy �� He cleans up at every iteration

    • May 19 at 02:14 PM

      The obsessive CLEARing is one of my pet peeves. I've seen too many times when a variable is cleared immediately after declaration.

  • Jun 04 at 03:21 PM

    I stumbled across function module SLS_MISC_GET_MEDIAN, which contains a brilliant algorithm to calculate the median (granted, the function module is marked as obsolete).

    Behold:

     IF NOT p_num[] IS INITIAL.
        loop at p_num.
          lv_num_count = lv_num_count + 1.
        endloop.
        sort p_num by number.
    
        lv_num_mod = lv_num_count mod 2.
        IF lv_num_mod = 0.
    *     --- even amount of numbers
          lv_index1 = lv_num_count / 2.
          lv_index2 = lv_num_count / 2 + 1.
          lv_num_count = 0.
          loop at p_num.
            lv_num_count = lv_num_count + 1.
            if   lv_num_count = lv_index1
              OR lv_num_count = lv_index2.
              lv_median = lv_median + p_num-number.
            endif.
          endloop.
          lv_median = lv_median / 2.
        ELSE.
    *     --- uneven amount of numbers
          if lv_num_count = 1.
            loop at p_num.
              lv_median = p_num-number.
             endloop.
          else.
            lv_index1 = ( lv_num_count - 1 ) / 2 + 1.
            lv_num_count = 0.
            loop at p_num.
              lv_num_count = lv_num_count + 1.
              if lv_num_count = lv_index1.
                lv_median = p_num-number.
              endif.
            endloop.
          endif.
        ENDIF.
    
      ELSE.
        raise List_Of_Number_Is_Empty.
      ENDIF.
    
      median = lv_median.
    • Jun 04 at 05:08 PM

      I guess this is an ancient code before READ TABLE was introduced.. ��

    • Jun 12 at 01:16 PM

      Holy cr@p! Truly a gem :-)

      It was created in 2000, READ TABLE was around for a looooong time by then.

  • Jun 16 at 08:09 PM
    INCLUDE MM06EFMP
    *     Verfügbarkeit überprüfen: SUBRC ungleich 0 wenn alles ok ist!!!
          PERFORM availability_check.

    (Translation: SUBRC not equal 0 when everything is OK)

    • Jun 17 at 07:19 AM

      Inverse logic ftw.

      Or born to fail?

      Perhaps the PROgrammer is an old C-language fox? Used subrc as a hybrid parameter and returns how many of whatever are available? Old habits...

  • Jun 24 at 11:43 AM

    OK someone please tell me what CHECK TBL_ZSD04-POSNR IN R_DELIVERIES means.

    As far as I can see it is a double negative so if the value is IN the range then in fact it is NOT in the range and so result is FALSE. What if the evaluation was a thousand lines away from the routines that filled the range?

    And if you call a database table ZSD04 how can anyone guess what that table is for except - it might be something to do with SD?

    And C_PICK_STATUS_1 - its somewhat different to C_PICK_STATUS_A but in both cases you have no idea that it means the delivery has not been picked yet.

  • Add comment
    10|10000 characters needed characters exceeded