Clean ABAP Book Review Part Four : Explorers on the Moon

[hardyp180]

Far and few,

Far and few,

Are the lands where the Jumblies live.

Their heads are green and their hands are blue,

And they went to sea in a sieve. And wrote Clean ABAP.

This blog is the concluding episode, 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 three parts of this book review can be found at

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

https://blogs.sap.com/2021/02/03/clean-abap-book-review-part-two-dirty-gertie-from-number-thirty/

https://blogs.sap.com/2021/03/08/clean-abap-book-review-part-three-the-generation-game/

If you have been following the previous blogs you will have noticed the focus has not been so much on how to write Clean ABAP Code as such but rather on various puppets that starred on television in the late nineteen seventies in the UK like Basil Brush and Bruce Forsyth.  They are a hard act to follow and so I have been thinking long and hard about who to use to close off this series and I have decided that the Clangers can step up to the mark.

Clangers Annual 1971

As you can see the Clangers are knitted mouse like creatures, that speak by whistling. When I was young I had it my mind that they lived on a planet in deep space, but later in life most people seem to be of the opinion that they live on the moon. Either way it is an airless world, and the craters are covered over with dustbin lids. Inside the surface of the planet are Iron Chickens and the Soup Dragon.

Chapter 12 – Unit Testing

• I am going to waffle on like nobody’s business here, as this is a very emotive subject. Until recently very few people have done any sort of unit testing in ABAP and even now most ABAP programmers vary between “too much like hard work” to “adds no value” to “it’s like jumping off a bridge”.


• Right off the bat it is claimed that a “single unit” is a single class. I disagree, I say it is a single routine usually, though I have written tests that are at application level and run through a whole bunch of classes. Even in the latter case though what is usually really being tested is one piece of logic – I do this when the piece of logic in question keeps going for a walk, from a private method of one class to a private method of a totally different class.


• One thing I will say is that the whole SHORT/MEDIUM/LONG and HARMLESS / DANGEROUS / CITICAL settings are a load of nonsense. If you find that you have created a unit test that is not SHORT and HARMLESS then get straight down to the brain doctor, as you have totally missed the point. That is why you have that ATC check which gives an error if there is any sort of database access in a unit test.


• There is a recommendation to have global abstract test classes which your local test classes can inherit from. I have been trying to think of any re-usable functionality which I currently keep having to define manually in each local test class but thus far I have not thought of anything.


• There is something which has always puzzled me as an old school ABAPer. This is the love inside SAP of having comments which influence program behaviour. To me something is either a keyword like FINAL or a comment which is only for information. It would seem odd to me to have the fact that a class is final controlled by a comment. Nonetheless in earlier versions of ABAP unit the DURATION and RISK LEVEL were defined using comments before they became keywords in later releases. Now we come to the concept of “test relations”. Here you specify what classes (or CDS views) are being tested by means of writing special comments just before the class definition. I tend to feel the problems with using comments to control things rather than keywords is that comments are free form and if you spell them wrong there is no way any sort of static check can warn you. I could be wrong, maybe SAP does do a static check on some comments, which is true would also be really odd – a syntax check on a comment.


• There is a presumption that no-one uses SE80 anymore. I will say that using ADT makes TDD a million times easier as you can auto-generate empty skeletons for local class definitions and implementations from a call to a local method which does not yet exist, something you cannot do from SE80.


• I was happy so see a big emphasis in structuring test methods using the GIVEN / WHEN / THEN. It is not mentioned but to give credit where it is due as I understand it this concept was created by Dan North under the name “Behaviour Driven Development”.


• As to the quote “the given step is implemented in the SETUP method”. No it is not. The SETUP provides data which is needed by every single test such as an instance of the class under test. Then in each actual test method is the GIVEN step which sets up input data specific to that particular test.


• The class under test can also be called the code under test and you keep the same acronym. That makes sense to me as you could in fact be testing a FORM routine as opposed to a method in which case there are no classes being tested.


• As to the quote “the class has no interfaces because is a legacy class” that makes no sense to me. Either the class has tests in which case it is not a legacy class, or it has no tests in which case it is a legacy class. Whether it exposes its public methods via interfaces is neither here nor there as to its legacy-ness. If you think about it using “legacy” to mean “old” is silly. You write a new class – using the time-based definition how long until it becomes a legacy class? Ten years? Ten months? Ten seconds? Whichever one you pick is going to be arbitrary.


• As to naming test classes and methods that is pretty much a value judgement – the book says name a test class based on common GIVEN/WHEN requirements and the test methods based on WHEN/THEN requirements. I would say that the name of the test class should say what IT is as in “IT IS A MONSTER GENERATOR” so the test class would be LTC_MONSTER_GENERATOR. Then the test methods should be all the things IT is supposed to do i.e. MAKE_SCARY_MONSTERS or NOT_MAKE_FLUFFY_BUNNIES – basically the list of business requirements you were given at the start and were then added to over time. Of course there may be no meaningful difference between the approach in the book and what I just suggested, using either you may end up with the exact same names.


• “Legacy Code where the tests are not understandable” – as I said, if it has tests it is not legacy code. Again what is meant by legacy code here is code which is older than some arbitrary time limit.


• There is a very interesting discussion on the finer points of designing tests to react to expected or unexpected exceptions. I noted to myself that it is quite ironic to be reading about this subject in such detail when most programmers in ABAP world don’t use exception classes yet, yet alone ABAP Unit.


• Moving on to the subject of how to replace real dependencies with test doubles the suggested method is constructor injection. I always used to do just that until the open SAP course on TDD at the start of 2018. There the SAP instructors explained that “dependency lookup” was the way to go, with the CUT getting all its dependencies from a factory, and the test class “injecting” the factory during the SETUP method. I had never heard of that concept, but it is brilliant, and I would now never do this any other way, so goodbye constructor injection forever.


• In regard to test doubles not having to implement every method of an interface the book mentions you can use PARTIALLY IMPLEMENTED. If it is a global interface there is a setting you can make in every method definition that says to pretend there is a blank implementation for that method if any given class does not implement that method. I imagine purists hate that because in production this is technically a contract violation – the class says to the outside world it offers the functionally offered by this public method, but it does not really. So PARTIALLY IMPLEMENTED which only works in test classes is the way to go.


• Sanity Checks – I define these as ASSERTIONS or equivalent scattered throughout the code to see if something totally unexpected is happening. This is sometimes called “defensive programming” and is deemed to be a Bad Thing as you end up with more checks than needed. I don’t care, I love sanity checks, and if they are actually coded as opposed to comments so much the better. Thus, I was happy bunny to see the “Clean ABAP Code” authors putting in some sanity checks.


• On a nit-picking note, I would say that if you did feel the urge to use constructor injection then in the SETUP method make it obvious you are passing in a test double by calling the variable MOCK_SENSOR or SENSOR_DOUBLE or some such.


• In regard to the ABAP Test Double Framework, I do not doubt its usefulness, but I would say it is not “Clean Code” in any way, shape or form as the syntax is so obscure that no reader would ever be able to understand it. You have to do everything backwards and thus as far as I am concerned the syntax is BONKERS. In fact I have a little confession to make. One night in the pub in Heidelberg I had ten steins of Wheat Beer and then an entire bottle of whiskey and then I took a load of LSD, and then someone hit me on the head with a baseball bat, and only after that did I write down some notes as to how calls to a possible ABAP Test Double Framework might work, dictated to me by the Purple Crocodiles that were coming out of the walls. Someone who works at SAP must have stolen those notes, that is the only possible explanation as to how the ATDF syntax was designed the way it is.


• In regard to the CL_OSQL_REPLACE where you can magically re-route a database query so that instead of the actual database table like VBAK the SQL query actually reads some sort of dummy database table filled with fake data, I do not like that idea one bit. As far as I am concerned that is still database access. In the automated “code pal” ATC checks there is one which raises an error if it finds any sort of database access in unit test code, and good job too.


• There is a section on Test Seams. That would not be a bad thing in and of itself, if the section was comprised mostly of swear words mixed with some sort of incoherent rant about how if you ever used TEST-SEAMS you would be murdered by shape shifting lizard people from another dimension. That’s how I would write such a section. Sadly instead here we get a description of how the framework can be used with no mention of lizard people at all as far as I could see which is a glaring omission in my eyes.


• “test seams can be invaluable in legacy code”. No. They are supposed to be some sort of “stepping stone” to doing things properly but as far as I am concerned it is more effort than wrapping whatever it is in a method, for less gain, and just as risky. Also in programming world stepping stones tend to become permanent fixtures along the “I will fix it later = I will never fix it” lines.


• Then we come to other alternatives to the abomination that is TEST-SEAMS. One is to have test flags in production code, so that it knows when it is being tested a la Volkswagen. Obviously that is really bad, and to my mind at least this is exactly the same as TEST-SEAMS. In both cases the production code has something inserted into it which clearly indicates that a test can happen and in such a case something different will occur than in the normal course of events. Production code should never have any idea that it could be possibly run in test mode. Volkswagen were forced to change their entire business model as a punishment for cheating in this manner. It would be really ironic if the transition to electric cars was kick started by criminal behaviour.


• Here comes another new concept which I was blissfully unaware of. This time the idea is to have a subclass which behaves differently to the production code. When the test is running the subclass is used instead of the real class. That’s terrible Muriel – it’s like a test double but for the business logic. I have never heard of anything worse in my life. Compared to that TEST-SEAMS are indeed good, but that is like saying that being thrown of the top of the Leaning Tower of Pisa is better than being thrown out of a jumbo jet because in the former case you hit the ground sooner.


• One fact mentioned is that the generic term “Test Seam” is supposed to refer to either a test double to stand in for a dependency or one of those testing subclasses to stand in for the business logic. In that regard the ABAP TEST-SEAM is named correctly. It can stand in for anything.


• In regard to test code needing to be as good as production code – if anything it needs to be better “clean code” wise because in some senses the value of test code as living documentation is just, if not more, important than its function as an automated regression test. It is going to get read a lot which means it is vital that it is understandable. With a lot of ATC checks you can determine if the check is applied to test classes as well as production classes and apart from the “magic number” check (test code is full of magic numbers) virtually all of the normal static code checks apply to test code as well. You do not want a unit test that has a twelve-level deep nested IF construct for example.


• As to “unit tests should not depend on the environment in which they run” very few people seem to understand this. The environment in which they run should be development which has an empty database when it comes to master and transaction data, and you should pretend there is no configuration data either. A unit test which will only work if there is specific data in the database is a useless unit test as it could break at random indicating an error where none exists. Yet some writers still say strange things like “this test will only work in the quality system”.


• In regard to not testing private methods I can see the logic. That forces your tests to mirror the structure of the current implementation of the application and prevents you from changing the internal structure of the application as then you have to re-do dozens of tests. Nonetheless if I find a private method in production which is giving totally the wrong result then I reserve the right to create a unit test for that private method until I have got it behaving itself. People may say I should write a unit test for the “inflection point” i.e. the higher-level public method which calls the faulty private method at some point but the question then becomes – does that obscure the actual purpose of the test? As I said before all this nit-picking seems somewhat ridiculous when a lot of ABAP programmers (99%) dispute the need for automated unit tests in the first place, or even if they agree with “don’t have the time”.


• I quite agree you can never get 100% test coverage and should not resort to silly tests just to increase this metric. A much more important aspect of test coverage that I have discovered is that it highlights branches in conditional logic that can never be reached no matter what. The static code check is not clever enough to spot such situations but sometimes when I am trying to hit 100% coverage on one method I realise that there is an unreachable branch and that is either a bug in the program or a branch that needs to be deleted just to simplify the program. No computer can tell you which one it is, you have to use your brain! Doctor! My Brain Hurts! It wiil have to come out. Out? Out of my head?


• Just to return to the subject of naming the test class meaningfully I am reminded of my boss. If I say that the Yorkie Chocolate Bar report is not working and the business requirements keep changing and nothing I do to fix it works and I want to jump off a cliff and I wish I had never written it in the first place he will say “What is IT?”. It is the same when writing a book for SAP Press – you are not allowed to say “it” or “that” you always have to specify what you are talking about. Thus when naming the test class I try to answer the “What is IT?” question. IT is the application and the test methods are the various things that IT does in order to meet the business requirements.

The Soup Dragon

In the same way the Earth’s core is full of molten rock, the core of the Clanger’s planet is filled with soup which is harvested by the Soup Dragon for the Clangers to eat. They also eat Blue String Pudding, and on the surface of the planet there are Cheese trees in addition to the Music Trees.

Chapter 13 – Huge Packages

• Packages are violently important in other languages like Java but (at first glance) not at all in ABAP i.e. you can have every single development object in a package called ZEVERYTHING and it would seem to make no difference.


• For a third-party software company developing an add-on to SAP suddenly packages are much more important as they to ensure their software can be deployed to any given SAP system and have all the dependencies the add-on needs. For everyone else no-one has ever seemed to care too much.


• Back in 19997where I work we had no idea what package to put anything in, so we took the lead from the consulting company we were paying half a million pounds a month to, as they surely must know what they are doing, So we ended up with packages called ZENMM and ZENSD and ZENHR and ZENFI and for applications which spanned more than one module (which was 99% of them) there was ZENGENERIC. Looking back now I cannot help but wonder what the “EN” was all about. Any given SAP application can be deployed in many languages via text symbols and the like.


• Anyway that was silly but did not do any apparent harm. I always knew in my heart of hearts that a proper package hierarchy was somehow important but all the blogs and what not I read just went right over my head.


• What is going to force the issue for a lot of organizations is abapGit. An online repository has a top-level package and the repository has to be self-contained otherwise it could not be deployed to another ABAP landscape. If it does need anything big that big thing has to be in another package which lives in another repository and the dependency is declared in the first package and the APACK manager makes sure all depended upon packages are loaded as well. That is pretty much how Java works as I understand it – you declare what packages you need at the start of a program in some sort of configuration file.


• To over-simplify things there are two main sorts of programs – ones which perform CRUD operations of business objects like sales orders or monsters, and interactive reports which give you a list of Monster Sales Orders and let you perform actions upon them like releasing billing blocks or dispatching Monsters to go and perform the evil deeds specified in the sales order.


• It has taken a very long while, but I have finally got my head around how both should be designed in a proper OO fashion following all the SOLID rules, using a mixture of local and global classes. It’s a sort of pyramid where the more application specific something is the higher up the pyramid it lives and the more likely it is to be local, the more generic and re-usable it is the lower down the pyramid and the more global it is. So at the bottom are domains and data elements which could conceivably be used all over the place, and then a database table comprised of those elements which could be used by multiple applications and then maybe a CDS consumption view specific to this application, and then the actual application program which has assorted local classes inside it and maybe re-uses some generic helper classes for some aspects.


• All well and good. You never build anything the exact way twice, but after a while you find yourself following similar designs when creating an application to plan pub crawls one week, and an application to identify and destroy lizard people the next week. So what I am looking for in life is some sort of general guidelines as to how many packages should be created per application and what should go where. For example if a data element is used in three utterly disparate applications should it live in some sort of low-level base package or should you have three identical data elements with different names in application specific packages? What’s the best design to allow you to create a main branch and a project branch in abapGit?


• Up till now I have seen people either not bothering at all with packages via the ZEVERYTHING or ZSD approach, or creating some sort of arbitrary classification hierarchy which “looks right” based on the business model at the high level and the ABAP components at the low level. Either way, the programmers are pretty much just guessing what to do, package wise.


• At the start of this chapter the authors say that if you get the package design wrong you are creating a mountain of problems for yourself later on, so I had incredibly high hopes that this chapter may have given me some sort of worked example of how to structure a package hierarchy for an application badly, and how to structure one properly, and what the benefit of the latter over the former was in concrete terms.


• One quote is “reusable objects are duplicated because that they were meant to be reused is not obvious”. How true that is. For example whoever developed the CL_RECA set of classes obviously intended them to be as re-usable as possible e.g. the generic message handler. However since all those fantastic re-usable utilities are in a package called “Real Estate” I would imagine lots of programmers would be reticent to use them in other applications, and so go and re-invent the wheel instead.


• I keep wittering on about how some inside SAP have a love of using comments to control program behavior as opposed to keywords in code or settings in some sort of configuration table (which equate to checkboxes in form-based tools like SE24). My other pet hate is sorting things alphabetically instead of logically but let’s leave that one aside for now. Anyway when the book starts describing the various types of package interfaces it raises the fun fact that you must rely on a naming convention to expose the purpose of a given package interface. Oh dear! In my experience as soon as you start relying on a naming convention for anything you are sunk. Getting people to stick to it is difficult enough and even if they try the “fat finger” syndrome will stuff some names up. It is rather like the fight about CDS view naming – alternate naming conventions are competition with the SAP recommended one because a lot of people do not like the “_I” prefix being used for more than one purpose. If you had some sort of setting (it would be an annotation for a CDS view) to indicate the purpose then there would be no need for a mass debate and a naming convention would become a “nice to have” rather than something vital to the future of the universe.


• I get the general idea of package restrictions. This very day I found some of my colleagues had been re-using a standard SAP function module (unreleased of course) which purported to return the number of calendar days between two dates. However it gave the wrong result because inside the function was logic stating that every year had exactly 360 days and every month had exactly 30 days. In fact that was not even incorrect – when used for its intended purpose which was calculating interest in Germany where I presume the interest amount has to be the same each month regardless of the length of the month. That function module belonged to a very specific package to do with financial interest calculating thingies and if there had been some way to stop idiots like us re-using something that “looked right” then we would not have got into trouble. The instant we tried to re-use it we would have got a syntax error like “Only for use in the German interest calculating package” and we would have been forced to do something radical like subtracting the start date from the end date.


• With proper package restrictions, and the package check active in an SAP system this sort of thing becomes possible. There was an explanation of all the moving parts that had to be set up – general restrictions, point-to-point access, use accesses and so on. After a while my head started to spin and my brain melted and dribbled out of my ears and because my head was spinning around so fast all the walls and fellow patrons in the pub were splattered with liquid brain. I am sorry to say I envisage a similar reaction from anybody trying to understand how package restrictions work. If a group of programmers were on the second day of a training course about how to use package interfaces and the class was invaded by a ravening pack of zombies the students would be 100% safe. That is because zombies eat the brains of the living, and in this case all the brains of the developers would have totally evaporated with a few hours of the course starting. In other words this is REALLY The quote from the book is “the package concept in ABAP is a bit more complex than necessary”.


• I read in some blogs a long time ago that lots of standard SAP packages do have the restriction checks defined and when you pull a big red lever and activate such restrictions system wide all your custom code goes into meltdown because everyone uses so many standard SAP objects from all over the place. The implication is that you should pull this lever and then be forced to change all your custom code so that it only uses what it “should”.


• For an external company I gather the idea is to have a 100% enclosed “Z” package which only references standard SAP objects in the BASIS, ABAP or “Business Suite Foundation” layers as those objects would exist in every SAP system, not only ECC but BW or CRM or whatever.


• For everyone else it would seem like changing the existing code to do the right thing would be an enormous effort for no (perceived) business benefit whatsoever which is why I imagine very few organisations activate such checks unless they are doing a greenfield implementation.


• I think what the book is saying is that the checks are active by default in a dormant sense from ABAP 7.31 and up, but do not cause any type of syntax error or even warning in custom code when you use a “naughty” object. You can however run some sort of package check on a Z package or object to see how naughty you are being and do something about it if you are so inclined.


• I mentioned earlier from time to time I get the feeling that the “Clean Code” is primarily aimed at developers working within SAP as sometimes the reader is pointed to websites that can only be accessed by SAP employees and there are comments like “however if you do not work for SAP”. This is not a bad thing in and of itself, it proves SAP is eating its own Clean Code Dog Food. BTW the phrase “Drinking Our Own Champagne” which SAP has come up with as alternative to “Eating Our Own Dog Food’ completely reverses the meaning of the original saying. It’s like taking the phrase “Casting Pearls before Swine” and changing it to “Casting Pearls before Jewellery Experts”. It sounds a bit the same, but the meaning is totally different. Just to hammer home the point Champagne looks and smells nice and thus you would have no qualms about drinking any type, no matter how poor quality. Conversely dog food looks and smells horrible and so it would have to be very good indeed, the best in the world, before you would eat it.


• Anyway I always laugh to myself when I get an extended check error telling me a standard SAP function module is obsolete. I cross my fingers and hope that the obsolete function module has some documentation telling me what to use instead. A class hopefully, but that never happens. If you are lucky you will get pointed to a replacement function but usually you get the comical instruction to “contact the person responsible for the function module”. Well that person worked for SAP and even if they are still there the chances of contacting them are zero.


• In the same way in the book there is the quote “if you’re not the owner of the object S, you can request access to it by asking the owner to publish it in a package interface”. That might make sense in a gigantic company like SAP but for most of us that makes no sense whatsoever since nobody “owns” any program or class or data element or whatever. There may be a loose rule that the last person who changed something is stuck with it until such time as they can offload it onto someone else. Thus if M. Mouse and is on holiday and D. Duck gets told “Mickey usually looks after the ABC report, but we need an urgent fix, please take care of it” and Donald makes that fix, then two months later when a change request comes in for the ABC report when someone asks Mickey he will say in a high pitched voice “Donald was the last one to touch it – he owns it now. I have been trying to get rid of that report for ten years! Bwa-Ha-Ha-Ha!”


• In regard to making design changes to fix package dependency errors. In the book they say one alternative if you are not allowed to use an object is to make an exact copy. I have always wondered about this. If four applications want to use the same data element and it has the exact same semantic meaning in them all, do you make four identical copies, or have the data element in some sort of shared DDIC package? I would guess the latter. What about when a data element is in one package and another application comes along and wants to use the same data element as the semantic meaning is the same? Do you move the data element down a level into a shared level? This question is even more important to classes and the like. You do not want to duplicate code, so do some potentially re-usable classes start off in a specialised package and then “sink” over time moving every so often into ever more generic packages?


• They say that cyclic dependencies between objects is bad and I agree. When asked for an example of such a thing I always use this one : Value Table depends on data elements. One of those data elements depends on a domain. That domain depends on the value table, which depends on the data element which depends on the domain which depends on the value table and so on. There is no way around that one as far as I can see. It is even worse in CDS views where you have a header and item table with an association to each other. You cannot write the final versions of each and activate them both at once. Each one will give you a syntax error saying the other does not exist. Obviously there is a way around this, but it is really painful.


• Suddenly I find myself at the end of the chapter about packages in the summary section. What I had scribbled at the end (I don’t scribble in an actual book but on a hard copy print out of the E-Book BTW) was “Where was the example?”


• There was some sort of set of alternative design approaches along the “application vs layer” lines but as far as I could see there was no worked example of how an application could be structured into different packages. This is a huge gap, not necessarily for a book on clean code, but for the knowledge of ABAP programmers in general. If someone wants to step up and do a blog, or series of blogs with a worked example, that would be great.

An Iron Chicken

 

The narrator claimed that claimed that in reality when the Clangers' were whistling, they were "swearing their little heads off”. The dustbin lids on the craters and the Roman style armour they wear are to protect against meteorites and space debris. And Iron Chickens.

 

Chapter 14 – Implementing Clean ABAP

• They say people spend more time reading code than writing code. That is 100% true. Think about sitting in front of a debugger all day and then finding the problem and changing one line of code. I did that this morning. In an editable CL_GUI_ALV_GRID I was getting a strange error message when the user tried to change the header billing block VBAK-FAKSK. When the “check changed data” method of the CL_GUI_ALV_GRID was called a funny error message popped up – something about the pricing procedure. After an hour or so of system debugging I discovered that when checking for changed data a whole bunch of fields were getting checked – most of which were not even editable. These lucky fields got evaluated as follows – firstly a check was done to see if the combination of table and field passed into the field catalogue e.g. VBAK-KALSM. If that was true then the check table attached to the table definition was read – table T683 in this case. Note that though VBAK-KALSM was in the internal table it is a display only field. Anyway the standard SAP code does a check to see if the value (pricing procedure in this case) exists in table T683 and if it does not up pops an error message and any changed fields are reverted. A strange check considering it is not the KALSM that got changed. Even stranger the pricing procedure DID exist in T683. However T683 has three keys – the first two are KVEWE & KAPPL. Neither of those fields were in the internal table being displayed so the standard program did not know what values to use so passed in blanks and so the entry was not found, hence the error. Anyway, I changed what was passed into the field catalogue form ‘VBAK’ ‘KALSM’ to ‘ ‘ ‘KALSM’ and suddenly it was possible to change the billing block. So two hours of debugging and then deleting four characters from the program and I am done. Someone somewhere once said they actually spend more time deleting code than writing new code, and if that is the case with you, that is a good thing, as it means you are adding new good things and deleting old rotten things.


• This chapter makes much of “collective code ownership” – I talked about this earlier. In gigantic companies like SAP the code base is just so big and there are ten thousand programmers, so it makes sense that people have to specialise. I came from the opposite situation - for many years I was one of only three programmers, and some companies only have one. In such situations everyone “owns” everything and as mentioned earlier deciding who fixes what is usually based on the “you touched it last” rule. When we had to go through ten billion warnings in our Z code based highlighted by a check tool at upgrade time we sorted all the programs alphabetically and each took a third of the list. I suppose most companies are somewhere between the two extremes highlighted, which is why the book is pushing for everyone behaving as if they were in the small situation with everyone owning everything.


• You need a common understanding of what clean code is and why it is important. That seems almost impossible. Some people think the extended program check is a load of nonsense and you would have thought something like an unused variable or not checking SY-SUBRC would be hard to argue with. We made clearing the extended program check errors manually in our (now quite big) programming team and I strongly suspect some people just put pragmas against everything to make the errors go away. Once upon a time in the year 2000 we had someone, and his job was to “look after” failed incoming IDOCS. It turned out he was looking after them by deleting them, that way at the end of each day there were zero IDOCs in error. We only found out when he went on holiday and someone else looked at the failed IDOCs and reported back to the programmers what the error messages were. It would be nice to think the original person just did not understand what he task was supposed to be, but I think he did, it was just that deleting things was easier than troubleshooting, in the same way adding pragmas is easier than seeing if the variable really is unused or if not checking SY-SUBRC could cause a problem.


• They mention the broken window effect and that is so true. If you are changing a program and do the SLIN and find three warnings then it takes no time to fix them. If there are 500 it could take days, so don’t bother, don’t even check the changes you just made, and that ensure the program has more errors and warnings then when you started. You could call that the “Evil Boy Scout” rule – instead of leaving the campground tidier then when you found it instead just before you leave throw bottles of petrol around until there is broken glass everywhere and then torch the place.


• Usually you can spot if there is something wrong with production because either (a) a load of short dumps start happening or (b) the SM66 list filles up the screen. But what if there are always hundreds of short dumps in production every day? What if the SM66 list is always full? It is difficult to spot if you are burning the toast if the kitchen itself is on fire. It is the same with code. One way to avoid this situation is not to set the kitchen on fire in the first place i.e. run static code checks on new programs. We had one code reviewer tearing his hair out because a program had 20 billion zillion extended program checks and it was not even live yet.


• To be fair the new program with all the errors and warnings had been copied from an existing program which often got used as a template. Now we have a new problem – this “template” program was always getting copied “warts and all”. So ideally someone should go through it and clean up all the warts, so they were not constantly replicated all over the place. However how do you justify to the powers that be that you want to make code changes to a program that works perfectly (the one being used as a template) when no business user has requested any change to it, and there are ten million competing priorities? You need management who think long term, easier said than done.


• A quote from the book is “later quite often means never”. I would say that later always means never as in “the deadline is tomorrow so I will send this program with errors and warnings all over it like a pinball machine into production now, and then later check all the static code check errors and warnings”. This translates to “the deadline is tomorrow so into production it goes, and I will never even think about it again unless we get an end user complaint at which point I will address that specific complaint only”.


• Apparently at Google when doing a code review you can add comments to the code prefixed with “NIT” which are purely educational and do not require any sort of action. I wonder if the “NIT” is short for “Nit-Picking”?


• I wonder if instead of everyone resenting the whole concept and wishing there was no extended program check and no code inspector and no clean code guidelines it could be possible to turn the whole exercise into some sort of game that people would enjoy playing. A vaguely similar example is that one day I had a solution looking for a problem – I had just discovered global temporary tables and wanted to see if they really did speed up reports as opposed to repeated use of FOR ALL ENTRIES


• So I picked a key user and asked her if any reports were taking a long time. Initially she could not think of any, then one day she ran a “Z” report and it took 13 minutes, at which point she pointed me in that direction. I managed to shave 95% off the runtime, and then that trigger a list of another three Z reports that caused grief. People in the office talk to each other and if someone mentions to a colleague their report has gone down from 13 minutes to 30 seconds then the colleague will remember that the next time they encounter a long running report and tell IT. The key here is to reward them for telling you rather than getting all resentful and thinking “Oh no! Another ticket! As if I did not have enough already!”.


• What I am happy to see referred to in the book is that it appears the management at SAP are not afraid to see what clever things other tech companies – like Google – are up to and try and emulate them rather than wallow in “not invented here” which was the traditional SAP approach. Ironically the two best speeches I have ever heard at SAP HQ in Walldorf were from managers from AWS and Microsoft. The Microsoft guy in particular gave a fantastic talk on “Agile” and what it meant to him but what made it really good was all the horror stories about teams at assorted companies he had visited who all thought they were “agile” but were doing nothing of the sort and then when it did not work blaming the “Agile” methodology they had followed. He did not use those exact words, he let the stories speak for themselves. Generally the higher the percentage of your presentation is in the form of a story or stories the better it will be, especially if you jump around the stage playing all the characters yourself using different voices.


• What is a Kata? He is an unstoppable machine. Oh hang on. No that is A Kata is a series of movements you do again and again until it becomes second nature and you can do whatever it is without thinking about it. That’s a martial arts thing usually, but as I type this I am not even looking at the keyboard, I don’t have to think about where the keys are after all this time. I would say good candidates for this in programming world are using ABAP in Eclipse, or the Test Drive Development process. Both are agony at the start but in both cases you want to get to the stage where you don’t even have to think about the mechanics - leaving your mind free to use those tools to solve your business problem woes via programming.


• When you try and introduce the idea of code reviews people get very nervous. They think it is going to be some sort of attack on them for writing rubbish code. It’s not about that at all, it should be a sort of 360 learning thing – the reviewer learns about the latest changes going on in the system and lots of different programs, the person being reviewed might get some useful tips provided all feedback is done properly.


• What I mean by that is that feedback should be like an error message in SAP telling the user something is wrong with the data they entered. It is very important that the error message does not call the end user an idiot. Ways to do call the user an idiot include the use of exclamations marks and upper case in the message text. Instead it should be in the form of a suggestion like “please enter a quantity above zero because ABC”.


• If the reviewer cannot understand the code at all it may not mean the code is bad functionality wise, but it needs to be changed so the next person who comes along can understand it. The original author can obviously understand it (you hope) at the moment because everything is crystal clear in their head but in six months’ time when the original author comes back to the code they will be just as confused as everyone else.


• Presumably the idea is that everyone in the team just gets better and better at programming forever. There is no magic plateau where you reach mega-genius level and can never improve thereafter. Therefore I am going to use the Toastmasters analogy. Toastmasters is all about public speaking and you can get better at this over time, but you can never be perfect. Just go to any conference (they are all virtual at the moment, but the point still stands) and think about why you are not utterly enthralled the instant every speaker opens their mouth and then hang on their every word and the 45 minute speech seems to only take a few minutes and you loved every second. I am sorry to say but most speeches at SAP conferences are not like that – though I have seen some that come close. It is what I always strive for but for most speakers it just doesn’t matter.


• In Toastmasters the ability to give constructive criticism about a speech you just heard is valued exactly equally with being able to give that speech in the first place. Thus you have evaluation contests where a group gets a mystery speaker they don’t know from another group (e.g. me) and I give a four-minute speech. Then each contestant gives an evaluation of my speech and tries to give me constructive criticism about how I could have done it better. The judges then vote on who gave the best evaluation and they win the prize.


• Toastmasters say a good evaluation is like a sandwich. You start by saying something positive that was good about the speech, then in the middle goes as many suggestions for improvement that you can think of, and end on another good thing about the speech. A code review could be like that, no negativity at all. I like to think I am fairly good at giving speeches by now, but I am nowhere as good as some of the others I see at Toastmasters and am not ashamed to admit it. I am trying to be better all the time, and that is all that matters.


• It’s the same with code. I like to think I am a good programmer, but when I am with the other SAP Mentors I have a serious case of impostor syndrome. Nonetheless as long as I am getting better at programming every single week I am content. If I looked at code that I wrote five years ago and thought it was great then that would be a terrible situation to be in. Luckily thus far when I look at such code I always say, “I must have had ten pints before I wrote that, what was I thinking!”


• So this is a journey we can all go on together and thus as Adam Ant said, “Prince Charming, Prince Charming, Code Reviews are nothing to be scared of”.

Conclusion

Clangers Table Cloth

As you can see the Clangers use flags that astronauts leave behind as their tablecloth. So I ask all you conspiracy theorists – if the moon landings were faked, how did the Clangers get their tablecloth then? A-ha! Don’t have an answer for that one, do you?

I asked the Clangers what they thought about the Clean ABAP book and they said:

“Whistle, whistle, whistle,

Whistle, whistle,

Whistle, whistle, whistle”

I could have not put it better myself and I hope you all agree. The only way you will really know for sure is if you read the book yourself.

Anyway, it’s Goodnight from Me, and it’s Goodnight from Him.

Boom Boom Mr.Roy!

It Could still be a Big Night – if you Play Your Cards Right! Goodnight!

Whistle Whistle Whistle!

Paul

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