ABAP Code From Hell
There are times as a SAP development consultant when you come across code that just makes no sense. You know – statements that are never reached, potential infinite loops, bad use of SELECT, performance hindering code, even just plain baffling. I like to call this type of code “crazy code”.
An experienced developer would agree that in general, if the “crazy code” works, he/she should leave it untouched – why? Simply because if you change someone else’s code that looks illogical but still works, you stand the chance of introducing new problems that you overlooked. Don’t get me wrong – I think all “crazy code” should be rewritten, but when you stumble upon it by chance and have no approval from the business to rewrite it, then leave it alone. (At least until you have convinced the powers that be that it needs to be rewritten before the code reaches its optimal degenerative potential)
There are of course times when the “crazy code” is the cause of problems found when testing, or even in a live environment. Over the years, there have been many occasions that called for a complete rewrite – sometimes an entire application, and other times just a few lines. I often stare at the code in amazement and wonder as to how someone actually thought it would work. Today is one of those days. The code I came across today amused me to such an extent that I thought it important to share with the community. In fact, I am currently working on an SDN wiki where you can contribute by sharing your “crazy code”experiences.
As an introduction to the future wiki of “ABAP Code – Bad to Better”, here is the gem that I came across – This was of course in the live environment of a client of mine, causing the leave application to hang under certain conditions…
For those readers not able to see the obvious blunders, I have added some comments to point out the poetic style of code.
*&---------------------------------------------------------------------* *& Form Z_SHIFT_WORKER *&---------------------------------------------------------------------* * Check if leave type appropriate for shift worker *----------------------------------------------------------------------* FORM z_shift_worker. flag = ' '. * Let’s put this code in a loop – just for fun. WHILE flag = ' '. "<< ??? * I'm only looking for one record, but lets go through * them all, just to make sure. SELECT * FROM pa0007 WHERE pernr = ipernr. "<< ??? * There's a chance we get here even if we don't find a record, right? IF sy-subrc = 0. "<< ??? * Why use the WHERE clause for BEGDA and ENDDA wen you * can just use an IF statement? IF gv_begda GE pa0007-begda. "<< ??? IF gv_endda LE pa0007-endda. "<< ??? *----------------------------------------------------------------------------- IF pa0007-schkz+0(4) = '12HR'. IF gv_subty = '0101'. MESSAGE e026. ENDIF. flag = 'X'. "found a valid record EXIT. ENDIF. *------------------------------------------------------------------------------ IF pa0007-schkz+0(3) = '8HR'. IF gv_subty = '0102'. MESSAGE e027. ENDIF. flag = 'X'. "found a valid record EXIT. ENDIF. *------------------------------------------------------------------------------- IF pa0007-schkz+0(3) = 'ART'. IF gv_subty = '0102'. MESSAGE e027. ENDIF. flag = 'X'. "found a valid record EXIT. ENDIF. *------------------------------------------------------------------------------- * If we don’t find a shift that starts with ART, 8HR or 12HR, then * the user deserves to stay in this nice little loop. Forever. ENDIF. ENDIF. ENDIF. ENDSELECT. ENDWHILE. ENDFORM. "Z_SHIFT_WORKER
The ABAP Code From Hell wiki will provide some entertainment, but will more seriously demonstrate badcode practices and sample how not to do things.
UPDATE: Thanks to UB’s suggestion, I have now create a new project on Code Exchange – why not join and earn points by suggesting better ways of improving bad code. Go to https://cw.sdn.sap.com/cw/groups/abap-code-bad-to-better
Best regards!
"IF 1 = 2" cannot always be labelled as crazy code". SAP deliberately uses it in BAPIs to track the messages 🙂
Cheers,
Suhas
It's interesting to come across a comment about the "IF 1 = 2" code. I have always thought that it was some sort of crazy way to prevent code from execution aside from commenting it out.
You mentioned that it is used in BAPIs to track the messages. Can you expound (or give example) on what you mean?
This is used for being able to use the "where-used" feature on messages, which are not raised, but returned as export variable:
es_bapiret-id = 'XY'.
es_bapiret-number = '000'.
if 1 = 2. message e000(xy). endif.
Because BAPIRET2 has such a crazy format:
TYPE, ID, NUMBER, MESSAGE_V1 to _V4
rather than
MSGTY, MSGID, MSGNO, MSGV1 to V4,
you can't even use
message e000(xy) into l_dummy.
move-corresponding syst to es_bapiret.
message e000(xy) into l_dummy.
move-corresponding syst to es_bapiret.
It shows that if 1 = 2. is not necessary. In my opinion using
message ... into lv_dummy
is a lot better. It fills the SY structure correctly.
message e000(xy) into lv_dummy.
es_bapiret-id = sy-msgid.
es_bapiret-number = sy-msgno.
es_bapiret-type = sy-msgty.
You got a wonderful "where used", filled SY and no misleading IF.
cheers Carsten
You should be careful with this statement.
1st not all code at SAP is written in ABAP.
2nd not all ABAP code is nonsense, rather, ABAP is a very powerful 4th generation language, though a little verbose. The problem rather is, that ABAP is little or not taught at universities, since it is a proprietary SAP language.
3d the original post was about customer code, not SAP delivered code. At SAP you get a thorough training of the language.
Best Regards, Thomas
That would lead to some kind of "bad example - good example" database that could be quite useful for learning purposes. Combining fun & learning...
Working as a development consultant I spent most of my time tumbling and stumbling through code like the one above or even worse.
Having experience with Java, C++, Pascal and even some x86 assembler I can say: There are bad programmers everywhere. It is not only ABAP or JavaScript or YouNameIt.
Sometimes I like to believe that they are beginners and are just "training" in a live environment, because of bad (or even none) mentoring. Sadly I have to admint that sometimes when I get ahold of the original developer I am completely shocked. Expecting someone in his mid twenties not much younger than me, maybe just starting his or her first job, I see guys around 40 who have been developing for over ten years.
It is kind of hard to tell them that their code is "not designed very well" and it is even harder to find out that they are hired external consultants just like myself. Who are on the project to fill in for the lack of knowledge on customer side.
This leads me to three conclusions:
1. It seems very easy for someone to call himself an ABAP developer. There seems to be less competiton than on other markets (like JAVA). Thus companies hire every consultant available. Compare the cost of a working hour of an ABAP and a JAVA developer, you will see.
2. In the SAP and ABAP world there are a lot of people developing who say of themselves that they are not developers "but they need to get things done and nobody else wanted to do it". Most often it is keyusers from the departments.
3. Old ABAPers do not like to change their way of doing things. Maybe it is only my surroundings... but do you know developers who prefer the old ABAP to the new ABAP/4 editor? Who do not know OO-principles? Who have not written a single ABAP OO class and still use reports and FMs?
cheers Carsten
"possibly because programming ABAP has not been treated as a craft as much as just a necessary evil in order to customize standard SAP functionality"
Most often than not, I get this feeling that ABAP is just a tool to customize SAP giving me the inclination to learn a "real" programming language such as Java or C#.
ABAP Pearls...
😉
Ciao
Andrea