Clean ABAP Book Review Part Two : Dirty Gertie from Number Thirty

[hardyp180]

“It’s her! It’s her! It’s Dirty Gertie from Number 30! Boom Boom Mr. Roy!” – Basil Brush

Basil Brush and Mr. Roy

This blog is a sequel, as it were, of an ongoing book review of I am doing of a new SAP Press book all about “clean” ABAP code. The first part of this book review can be found at

https://blogs.sap.com/2021/01/10/clean-abap-book-review-part-01/

…. In which I talked about the first three chapters. This blog talks about the next three chapters. As might be imagined this series of blogs is going to take some time.

In another sense this is also a sequel to a blog I wrote eight years ago

https://blogs.sap.com/2013/06/17/are-you-writing-dirty-filthy-abap-code/

Going back to more vital matters, at the end of each “Basil Brush” show in the late 1970’s Mr. Roy (pictured) would read a story and get constantly interrupted by posh puppet Basil Brush. One recurring character from those stories was “Dirty Gertie from Number Thirty”. She obviously took no care with her code, that must have been what they meant.

Fast forward to 2021 and the lady from Number Thirty could have a look at the new SAP Press book which is all about “clean” ABAP code.

Disclaimer vs Kramer

Firstly as mentioned in the last blog I get a free copy of the book from SAP Press to review. They have no control over the lunatic way I conduct such a book review, however. I am also most likely going to lose focus in the near future due to starting work on the fourth edition of “ABAP to the Future”  a.k.a. “ABAP Moon Rising” which is going to be a gothic horror story. I also like to write my SAP related blogs in the local Indian Restaurant hence the term “Rice with SAP” which the CEO of SAP has also started using recently.

Overview to a Kill

In essence the clean ABAP book is going through the condensed list on the clean ABAP style guide internet site and giving long justifications as to why each rule is good, rather than just stating the rules which is what the web site, by its very nature, does.

The other day we had a meeting at work of all the developers and the question of “optional” checks like the extended syntax check and the code inspector came up and I threw in the clean ABAP website for good measure. The reality is that it is difficult enough to get people to care about the information messages in the extended program check, let alone anything else, and they just laugh at the “CHECK must be in a certain place”

Now is the time to explain what “technical debt” is. There are about half a billion different definitions, but I am going to say that it is taking the “quick and dirty” approach to programming. The upside is that you get whatever it is working as fast as possible, but the downside is that with “quick and dirty” you have to pay interest in the form of extra time to maintain the program plus bonus unexpected bugs each time a change is made for the next twenty or thirty years.

To paraphrase what is mentioned in the book the concept of “Clean Code” does not seek to lower the time taken to write new code, but rather to enhance understandability and readability so the overall time taken in the 95% of the life cycle of an application that is maintenance is drastically reduced. I would say this is the opposite of technical debt – you are making a technical investment. It’s like spending a thousand pounds buying shares in a company (say SAP). In the short term you have a thousand pounds less with which to do important things like drinking. However in the long term you get dividends each year which eventually add up to more than what you paid originally plus your original investment is worth a lot more than you paid for it should you wish to sell it. However, very few people take the long-term view on anything. So what this “clean code” nonsense is about is looking forward to beyond the next ten seconds … if you dare!

Basil Brush on a Game Show

https://edinburghfestival.list.co.uk/article/109496-basil-brush-i-mainly-like-tea-shops-i-love-dunki...

Chapter 4 – Madness in my Methods

• Whilst talking about OO programming in general the observation is made that static methods are not OO at all. Sometimes you need them but really they are just better versions of function modules. However when people start with OO they like static methods because you do not need to create an instance of the class. I know I felt that way right at the start.


• On the website a stated rule can seem like “Do this thing. Do not question it, it is best practise”. When you have a chance to delve into the rule more deeply sometimes it becomes apparent we are talking about philosophy more than “Do X then Y then Z”. An example rule is the “Have less than 3 input parameters in your methods”. That starts off ad an edict but then you get the more detailed instruction to group related parameters into a structure or data object. Then you find you cannot do such a thing due to the fact the ten input parameters are all totally unrelated to each other. That is a huge clue the method does more than one thing and needs to split up – so trying to follow the rule has forced you to improve the design of your program.


• It is the same with the “no optional parameters rule” – for years I have been using an IF_DARK parameter to suppress messages (in F4 processing for example) and it turns out that is the very example used for what not to do. The idea is to have two separate methods. I wasn’t convinced but it made me think – it would be crazy to have two methods with the same business logic, one with messages, one without. Then it came to me what I could do is have the parameter as a message outputting object and pass in either a real instance that outputs messages or a subclass which does not. That would help with unit testing as well, you could have yet another implementation which collected the messages for later evaluation by the unit test.


• Throughout the book there are some good examples where the code gets better through several different versions. It is also very clear as to why the new version is better each time. Some critics of OO would jump up and down and scream blue murder that “the code gets longer as it gets better” the inference being that the longer version is not better at all, because it is longer. I would say that is all is well if the end result is that the end code is longer in that there are more lines of code, but clearer to understand. That is the exact reverse of changing code to use some of the new syntax constructs!


• One thing I thought really ironic was an example of a method with three really horrible exporting parameters – a table containing the results of the method, a table containing the keys of any failed records and a table containing the actual error messages. That is incredibly convoluted especially considering that for 30 years we have been used to a single RETURN table coming back from a BAPI. The strange approach with the three export parameters is, amazingly,  just exactly what the RAP does. I suspect that is why you do not have to declare the EXPORTING parameters in the signature of a RAP method they just sit there invisibly i.e. the RAP hides the “dirty” parameters which is brushing the problem under the carpet in my mind.


• There was a huge explanation as to why EXPORTING parameters should be pass-by-value as opposed to pass-by-reference. Even after all these years a lot of programmers’ struggle to grasp the difference. In essence, if you have EXPORTING parameters as pass-by-value then you do not have to remember to clear them at the start of the routine.a


• Here is another rule “All methods should be five statements or less”. Now that is easier said than done and I would guess many programmers would just laugh at that guideline and say it was impossible. I read a blog by Robert Martin once where he did just that and atomised a large method into a tree of really small methods. As is often the case with logs the comments can be just as interesting (sometimes more so) than the actual blog. In this case he pushed a lot of emotional buttons as some people went insane with rage at the very idea. I recall someone screaming that “Uncle Bob” was the Emperor with No Clothes on at all and this “short method” thing just proved it, and all the people who hero-worshipped him were thick as three short planks. I tend to think he is wearing clothes, which is just as well really.


• One thing I noticed just this week is how exception handling can come to dominate the logic of any given routine. You start off thinking something is straightforward and then before you know it a multitude of edge cases crop up and you have to check them all, and before you know it for every line of business logic there are ten lines of error handling code looking for unusual situations. Once again the proposed solution is to isolate each such check in its own method. Robert Martin (however you may perceive him) once said that methods should do one thing only and exception handling counts as “one thing”. He is bound to keep cropping up in any discussion of Clean Code as he literally wrote the book on the subject.

Basil Brush with ABBA

What does Basil Brush expect to come back when he throws it? A Boom-Boomerang!

Chapter 5 – Names

• Just to drive home the point that no-one (in programming world) ever agrees about anything I tend to disagree with the name of the term they use to talk about naming things. It is claimed the rules presented about what to call things are “naming conventions”. I would say a “convention” is to mandate the something e.g. classes must start with ZCL_SD_ or data retrieval methods should start with DERIVE. This chapter is more about an abstract concept i.e. “make names of anything clear to understand”


• Last week at work I finally got to the bottom of something that had been confusing me for over a year. I just could not work out where a value was coming from in an old (enormous) program. It turned out the programmer had used the “Hitchhiker” technique. They deliberately put the wrong value into a structure representing a sales order item. More specifically the PSTYV (item category) was filled with the item category of the reference document which was just plain wrong. This was done purely to enable that value to be transferred into another routine. Upon arrival at its destination the wrong PSTYV value was transferred into another variable, and the order item PSTYV blanked out and then filled with the correct value. That worked OK but it meant that between picking up the hitchhiker and dropping them off many subroutines later the data had a deliberately incorrect value in it. At any stage on that journey some part of the program might have decided to evaluate that incorrect value and take a decision based upon it, possibly leading to disaster. Anyway, the moral of the story is that if you follow the rule that you always call things what they actually are then you would bar yourself from performing such sneaky hitchhiker tricks.


• Often in old (or even new) programs there are half a hundred routines for reading data from the database or calculating some bit of data based on other values. Each routine will start with a different name e.g. READ, CALCULATE, GET, SET and a million other variation. The idea is to settle on one and stick to it throughout the program. I tend to prefer DETERMINE instead of READ is because that matches the terminology in the BOPF and the RAP i.e. both of those frameworks have special methods called determinations and validations. So I use VALIDATE instead of CHECK for the same reason.


• I am always wary of “must” rules as in “the prefixes that must be used include the following”. Whenever someone says, “You must do this!” the instant knee-jerk response is “Oh really? Must I?”


• One such “you must follow this convention” rule is (for company ACME) to use the names ACMED for transparent table and ACMET for table type. That’s a value judgement surely? Lots of people use T for transparent table and TT for table type.


• Then on the very next page the proposed mandatory naming convention changes. Now suddenly T means transactional table, D is a data table, C is a customising table and “I” is a system table. Also if C means customising table, why do all the SAP customising tables start with a “T” e.g. T001W?


• One suggestion that took me by surprise was not to prefix classes i.e. do not use ZCL for global classes or LCL for local classes, just use ZMONSTER for a global class or MONSTER for a local class. The idea is that if named well it is obvious it is a class, and the prefix just eats up letters. Of course one way around this whole thing would be for SAP to expand the allowable length but that is probably never going to happen.


• I may have mentioned earlier that the definition of “legacy code” in this book is not the one I use. I say “legacy code” is code without tests. In the Clean book the term seems to refer to old code. Moreover this old code is presumed to be OO code as in “apply the new naming convention to everything except public methods”. A load of books down the ages have operated on the presumption 100% of the existing code in a system is written in an OO manner, and none in a procedural manner, and in real life the opposite is true in 99% of companies that run SAP.

Basil Brush in Pantomime

'My problem is I can’t tie myself down to one girl... because I can’t do knots. Boom! Boom!’

Chapter 6 – Variables and Literals

• Sometimes the new syntax is proposed as a solution to a common problem and the inference is that this is “clean” and clear and readable. Here comes an actual example from the book:


• DATA(buffered_partners) = FILTER #( it_partner EXCEPT IN partner_store WHERE ID = ID )


• That is not clear or readable. That is utter gobbledegook. That was written by Dirty Gertie from Number Thirty on her off day.


• One rule is not to do inline declarations inside IF constructs or LOOPS on the grounds that might confuse a programmer who was used to other languages as the scope of inline declarations in ABAP is different than that in, say, Java. It could of course be argued that lots of things in ABAP would confuse a Java programmer and vice versa. Still I suppose consistency here is one less thing to worry about in such cases.


• In regard to scattering data declarations throughout a big routine. Routines are supposed to be small but lots of existing ones are huge, and the claim is that scattering data declarations throughout a routine would make the code harder to read and understand. Many people would argue the exact opposite and indeed, prior to inline declarations a movement had started to move from declaring everything at the start of the routine to declaring things just before they were used. Inline declarations seemed to validate that approach. They are in some senses data declarations scattered throughout the routine.


• I have never actually used the keywords REFERENCE INTO ever in a LOOP (or anywhere else). So I am happy to see the recommendation is not to do this but to stick with FIELD-SYMBOLS.


• When I saw the recommendation online in the style guide not to use “constant interfaces” but rather enumerations I thought they were talking about the ABAP ENUMERATION keyword which is only available in releases higher than most organisations are yet on. I had got totally the wrong end of the stick – the book explains the concept of an “enumeration class’ which anyone can use. I had never heard of such things before, and I think the idea is wonderful. I am not going to tell you anything more here, I don’t wish to parrot all the good bits from the book in a review. I will say the idea of doing a unit test on a constant was a new one to me, it sounds BONKERS at first, but now I think it makes so much sense.


• If I had my nit-picking hat on I would say an ABAP message type of “S” means a “success” message as opposed to a “system” message which is what is claimed in the book.


• I also disagree with the comment that a constant structure named MSGID_V1 makes it obvious what the values inside refer to. I could not work it out, I has to wait for the explanation.


• Who would have thought that the inline declaration DATA(string) = ‘Hello I am a String’ would have created a variable typed as CHAR as opposed to a string? It was news to me. If you use so called `back ticks` or |pipes| then the result would be a string.


• It is claimed using |pipes| however has terrible performance problems. Does it? I hope not, I use them all the time. Has anyone reading this got into performance trouble by using lots of pipes?


• ABAP_UNDEFINED – I have never used this; it never made any sense. (It is a Schrodinger’s Cat state which is neither true nor false till you open the box). The book explains how it could possibly make sense but then says never to use it, which is fine by me.


• I would change the statement “regular expressions are often hard to read in program code” to “regular expressions are always hard to read in program code” and if you don’t believe me you can [0.9]{A-Z0-9_/]{0.20} and see if I care.

Basil Brush presenting Breakfast Television

Basil is 458 in fox years but looks remarkably well on it. He doesn’t have a single wrinkle.

‘To make myself look younger I put cucumber over my eyes, tuna over my ears and a bit of lettuce in front of my face.

'It doesn’t do any good, but it reminds me of my salad days. Boom! Boom!’

Conclusion

According to Basil Brush there are two sorts of programmers – Clean Coders and Dirty Gertie from Number Thirty. Clean Code is not what people think it is – it has nothing to do with functional correctness or performance or indeed compact code i.e. the sort of things the standard static code checks generally look for.

What it’s all about is making he code clear and understandable so someone can come along and look at it in ten months’ time and instantly know what the code does and what bit to change.

Imagine that Dirty Gertie and Basil Brush are having a race. As soon as Karl Marx’s sister, Onya, fires the starting pistol, off runs Dirty Gertie as fast as she can. Meanwhile Basil Brush climbs into a sports car. It takes him half an hour to get in, as he is only one foot tall. Nonetheless once he is in, he puts his foot on the accelerator and rockets past Dirty Gertie at a hundred miles an hour.

In other words, writing Clean Code shortens the overall time needed to work on a program throughout its life cycle at the cost of lengthening the time needed to write new code. Since 90%+ of a program’s lifecycle is maintenance it does not take long to pay for itself. To my mind that is just like adding unit tests.

There are about thirteen chapters in the ABAP Clean Code and so I will be writing one or two more blogs on the subject soon.

Cheersy Cheers,

Paul

https://www.sap-press.com/clean-abap_5190/