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.

90 Comments

  • Feb 18, 2020 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, 2020 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, 2020 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, 2020 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, 2020 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, 2020 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, 2020 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, 2020 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, 2020 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, 2020 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, 2020 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, 2020 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, 2020 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, 2020 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, 2020 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, 2020 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...

    • Jan 12 at 08:37 AM

      For my shame, I had a coding period like that. With ABAP. Then, somehow, I saw the light and begun to call my variables with real world words, moving from

      LOOP AT t_afko INTO w_afko WHERE aufnr = l_aufnr.
      

      to

      loop at orders into data(order) where number = pordernumber.

      I'm still trying to find a way to differentiate parameters from local variables. But the point is most of my code can be read by a monkey (or even a FI consultant).

      • Jan 12 at 10:04 AM
        Loop at Orders_Header
             reference into data(order_header)
             where order_number = order_number_requested.
        endloop.

        Maybe like that ?

      • Jan 12 at 05:10 PM

        I'm still trying to find a way to distinguis (?) parameters from local variables.

        Most of the time it's obvious, and most of the time I use RETURNING values that are usually called result.

        But I admit for importing/exporting I still use i_ and e_, it's use is so widespread that something like requested_thingy is less clear.

  • Mar 17, 2020 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, 2020 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, 2020 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, 2020 at 06:07 AM

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

    Can you spot what is wrong?

    • Mar 31, 2020 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, 2020 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, 2020 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, 2020 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, 2020 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, 2020 at 12:05 PM

      I'll join your bet.

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

    • Apr 03, 2020 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, 2020 at 07:57 AM

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

  • Apr 06, 2020 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, 2020 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, 2020 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, 2020 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, 2020 at 11:40 AM

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

    • Apr 13, 2020 at 04:05 PM

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

      • Apr 13, 2020 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, 2020 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, 2020 at 02:58 PM

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

  • Apr 23, 2020 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, 2020 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, 2020 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, 2020 at 03:37 PM

    A piece of code from the fifth of BIG4

    Very clean boy �� He cleans up at every iteration

    • May 19, 2020 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, 2020 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, 2020 at 05:08 PM

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

    • Jun 12, 2020 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, 2020 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, 2020 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, 2020 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.

  • Aug 04, 2020 at 06:27 AM

    A standard one

    FM ATP_HUBER

    * Change value to trigger different operation
    * value 1 - ATP server (default)
    * value 2 - old check
    * value 3 - both and compare
    * value 4 - ATP in APO (prototype)
    STATICS: s_atp_type(1) TYPE c VALUE '1'.
      atp_type = s_atp_type.
    
    * value 1 - dialog intgrated in AVAILABILITY_CHECK_CONTROLLER (default)
    * value 2 - old dialog
    STATICS: s_atp_dialog(1) TYPE c VALUE '1'.
      atp_dialog = s_atp_dialog.
    
    * Change value to trigger different operation in SD
    * value 1 - > 4.5
    * value 2 - old check   4.00 - 4.50
    STATICS: s_atp_type_2(1) TYPE c VALUE '1'.
      atp_type_2 = s_atp_type_2.

    but why ?!

  • Aug 04, 2020 at 12:51 PM

    Last week in the #ABAPFreakShow I was asked how old my JSON document class is. I had no idea, but remembered, that somewhere in the deep of every Standard ERP system the class was used. But how to find this class?

    Easy, go to the SAP Service Marketplace and search for "fetzer". You will find a correction for this class with the class name in it:

    In the class /SDF/CL_E2E_XI_ALERT_JSON_DOC in your system you can find the COPYRIGHT method:

    -> the class is still in every R/3 system and has its 10th aniversary this year :-)
    (don't know whether it's still in S/4HANA, can someone have a look?)

    scn1.jpg (35.7 kB)
    scn2.jpg (17.7 kB)
  • Aug 26, 2020 at 12:53 PM

    Well you can't say we weren't warned.

    """"""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""
    
    """" !! IMPORTANT !! IMPORTANT !! IMPORTANT !! IMPORTANT !! """"
    
    """"""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""
    " This status check assists the check in RADSHPBOSTATUS!       "
    " If you remove or change this check, syntax or runtime errors "
    " in RADSHPBO will lead to an unusable system.                 "
    " You won't even be able to logon to the system and it will    "
    " be your task to access the database directly and fix it!     "
    
    """"""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""
    """""""""" !! ALL HOPE ABANDON, YE WHO ENTER HERE !! """""""""""
    """"""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""
    FORM is_global_switch_on CHANGING global_switch_is_on TYPE abap_bool.
     DATA: dshconfig TYPE dshconfig.
      CLEAR: global_switch_is_on.
    
     SELECT SINGLE * FROM dshconfig INTO dshconfig WHERE param_key = 'SAYT_KILL_SWITCH'.
      IF dshconfig-param_value IS INITIAL.
       global_switch_is_on = abap_true.
      ENDIF.
    ENDFORM.
    
    • Aug 26, 2020 at 01:23 PM

      Quite explicit way to say "do not touch"! IIRC I happened upon this warning - or a very similar one - during some debugging session or other.

  • Sep 09, 2020 at 10:13 AM

    I actually quite like this one, but thought I would share it nonetheless.

    Does anyone wish to give a response to this?

  • Jan 11 at 04:05 AM

    Today's gem is all to do with giving things clear names.

    I am sure it is obvious to everybody what ZZBS2_TKNU means.

    Just in case you did not guess for some strange reason the business meaning is "flag to say order quantity is entered in number of truck loads".

    bs2-tknu.png (14.9 kB)
  • Feb 25 at 11:47 PM

    I was just maintaining a program and the programmer had defined a variable to handle a return code from a function module as data element SUBRC.

    I tried to pass that into a routine that wanted SY-SUBRC and got a syntax error.

    Guess what the type and length of data element SUBRC are?

    Almost as much fun as trying to type a plant as WERKS or a storage location as LGORT!

  • Feb 26 at 08:45 AM

    I have a big surprise this morning, a colleague tries to modify the code.

    The objective here is to get the lowest value of a table.

    The logic is simple, if no line, no return, if one line, return the line. If more than one line, make a sort & get the first line.

    But the first line does not really work because there was a line with a quantity = 0.

    So my colleague decides to take line n° 2

    :)

    prog.jpg (59.3 kB)
  • 5 days ago

    He is a trick for young players. It is often easy to pick an SAP function module that does whatever it is you are trying to do, most are unreleased, what does that matter?

    You would think this function module returns the numbers of days between the two dates. How wrong you would be. In actual fact it returns the number of days between the two dates in an alternate world where every month has 30 days and every year has 360 days. Life is much simpler on that planet.

    I am going to replace this with LV_DIFF = LV_DATEFROM - LV_DATETO.

    • 4 days ago

      I really don't understand why a lot of developers are focused on the FM.

      As it is the truth, the good way to code. Your simple line of code VS this FM is a good example.

      • 4 days ago

        Judging from the number of questions "Is there a function module for..." a lot of inexperienced programmers haven't got a grip of the ABAP language, don't know how to search the documentation but do know how to use CALL FUNCTION

        • 3 days ago

          Well, it's also possible that they are told to re-use SAP code, except they just don't know (yet) where this applies and where it's actually better not to do that. Similar thing with reusing standard data elements. How many developers still believe that creating a custom domain/data type is some kind of sin in SAP?

          • 2 days ago

            I was told at a previous job not to create them as it just creates more objects. I pointed out that SAP could change the data element at anytime. They still didn't believe me. I wonder what they eventually thought when all the tables they used werks in - no longer worked. (Yes, it's been awhile. I can't even remember what version we were on)

            • 2 days ago

              I remember declaring something with type WERKS just to realize later that it's actually a structure and WERKS_D is the data element. :) I bet for all those companies in the reusing SAP data types business the chickens will come home to roost in the S/4HANA migration. This could've been easily prevented.

            • yesterday

              Like Bärbel Winkler, in my current project, they have re-used a lot of data elements, and a part of them does not exist anymore in S/4.

              I think about starting using variable LIKE table-field,

              And I see a lot of time, the names of column in specific tables starting with Z . Whyyy ?!

              (sorry to spoil your post Paul Hardy, but it helps me to speak about that :) )

    • 3 days ago

      I tested the above FM to make sure that it is returning difference of dates in alternate world. And you are right there is another world which we may never see.
      Luckily we can ditch the FM after knowing about this and the simple line of code is very useful.

  • 4 days ago
    "SY-SUBRC NE 0 is error

    What, really?

  • Add a comment
    10|10000 characters needed characters exceeded