Jimi Hendrix and his Journey to the Centre of the Earth
Jimi Hendrix and his Journey to the Centre of the Earth
Anyone who lived in the UK in the late 1980’s knows that in 1970 rock star Jimi Hendrix faked his own death, and thereafter embarked on an amazing journey to the centre of the earth. It’s been almost fifty years now, and he is about half way down, as shown in the below diagram, looking younger than he ever did due to the rejuvenating properties of the atmosphere breathed by the dinosaurs and lizard men at that depth.
Figure 1: Jimi Hendrix Progress Report
He would have got further, but the series of incredible adventures he has had unfortunately slowed him down somewhat.
Now, by this point I suspect a small proportion of you reading this are wondering what the link is to SAP in general and ABAP development in particular. Is it, you may be thinking, that JH has been whiling away his spare time on his journey reading up on CDS views and the like?
Don’t be ridiculous. Wi-Fi connectivity is no good down there, and anyway he is far too busy battling mud-men and the like.
I have about as much musical ability as a doorknob, but I too have been on an amazing SAP journey over the last five years, and as the Carpenters might say “Its Only Just Begun”.
To get some context here I need to “revisit” assorted blogs which could be seen as stepping stones on this journey. What this is all about is how to merge disparate SAP systems in various countries, all written in procedural code, all a “big ball of mud” into a lovely shining OO thing of beauty which fits all countries perfectly, and everyone lives in paradise forever more, and there is world peace also.
Once again, some may say that is a trifle optimistic. However as JFK said about flying to the moon “we do not do this, because it is easy, but because it is impossible”. And then they did it anyway.
Back in May 2012 I was returning to Australia, all too horribly aware of the difficulty of the problem (multinational code that is, not flying to the moon). Hence I reached out to the SCN community explaining the nature of the beast, and asking for advice. This can be seen in the following blog:-
By this stage I was devouring all the “classical” books and articles on OO programming, all of which was news to me. To quote from a Rolling Stones song about children “doing things I used to do, they think are new” to turn this on its head all my problems had been encountered before, and solutions were there, if not always, or indeed often, implemented in a successful way in real life.
An example is the so called “Template Pattern”. I had no idea what that was, but it did not stop me re-inventing the wheel and describing something very similar in October 2012 in the following blog:-
Time went by. Time, time, time. See what’s become of me, while I looked around for my programming possibilities, I was so hard to please.
But look around – leaves are brown, and the solution was a hazy shade of unit testing.
It sounds trite, but I honestly believe that making the “ball of mud” code testable was the solution to all my woes. Here is a blog I wrote in April 2013 on the subject.
Test, Test, Test
Figure 2: Mr. Tickle
The logic is that if I wanted to alter the horrible existing programs to make them portable to assorted countries and also much more stable, then changing them into a testable state was not only a good first step, but also would take me 90% down the road I wished to follow.
In my blogs and indeed at the start of the unit testing chapter of my SAP book I quote Michael Feathers as follows:-
“Code without tests is bad code. It doesn’t matter how well written it is; it doesn’t matter how pretty or object-oriented or well-encapsulated it is. With tests, we can change the behavior of our code quickly and verifiably. Without them, we really don’t know if our code is getting better or worse.”
― Michael C. Feathers, Working Effectively with Legacy Code
I cannot quote that often enough. Adding unit tests to my own code in my day to day job has helped me no end.
Now, as it turned out, it looked like I was actually going to be working with Michael Feathers earlier this year, arranged by an Australian consultant. Sadly it came to nothing, due to red tape at the target company (their loss, I would have to say, they lost the benefit of my enormous mega-genius, the greatest intellect in the history of all the possible alternate universes, and also the benefit of Michael Feathers) but at least I got to talk to him on the phone, and he said my chapter on unit testing was OK, as opposed to launching into a thirty minute barrage of swear words, which could have been a possible reaction.
I would advocate everyone to buy and read “Working with Legacy Code” (2004) by Michael Feathers, even though it is all about non ABAP languages, the principles are the same.
If you are feeling a bit stingy, then Google the terms “Michael Feathers Object Mentors Legacy Code” and you will most likely get a PDF summary of the salient points – but that is naughty.
Moreover, none of the examples are in ABAP, because – and it will horrify some people to hear this – ABAP code is actually a very small percentage of the code written every day. ABAP may be (and has been for ages) in use at the big companies, but there are a lot more small companies than big companies, and going forward in the SAP world ABAP will most likely play a smaller role, due to UI5 and S/4 HANA in the cloud. I cannot say that fills me with joy, but you have to live in the future, whatever it may look like, rather than running around in circles screaming “mobile phones will never catch on” like Stephen King.
In any event, since Michaels’ strategy is by no means secret (I am basing that statement on the grounds that anything that has been all over the internet for 13 years cannot really be said to be secret in any meaningful sense) then I am going to restate that strategy as it applies to ABAP code.
The Feather Girls
Figure 3 – The Feather Girls
Hi! Hi! We’re your Feather Girls – and have WE got NEWS for YOU!
In this case the news relates to what to do whenever you have a change request relating to a monolithic Z procedural program with no tests, full of global variables, and so on.
ABAP Legacy Management Strategy
- Identify Change Points
- Find an inflection point
- Cover the inflection point
- Break external dependencies
- Break internal dependencies
- Write tests
- Make Changes
- Refactor the Covered Code
- Identify Change Points
If the scope change wants you to change several behaviours, do one at a time.
Where do you need to make the change? It could be lots of places. However if you have to make the exact same change in lots of different places, then this is the time to encapsulate the code. It is a sure-fire sign of dangerous design.
From a test perspective, you want to really only test one area when making a change. If you have to write lots of tests for different areas affected by the change than that is a Bad Thing.
The worst possible situation is where there are fifty four almost identical blocks of code with subtle differences between them. A lot of people consider this too dangerous to change. If you have the guts the answer is to abstract the differences to parameters.
- Find an inflection point
An “inflection point” is the closest level in the call stack which will be affected by the change and then move it on to the rest of the program.
This is actually easier to spot in procedural programming than in OO programming, but in both cases in SAP world the “where-used” list is the key here. Let us say a “routine” is something that can be called by another art of the program i.e. a “routine” is a function module, a FORM routine or a public method of a local or global class.
If I make a change in “Get Alien Eye Count” be that a method of a class, or a FORM routine if that is only used by “Get Alien Details” then the changed routine is not the inflection point. In the same way if “Get Alien Details” is only used by one higher level routine then it is not the inflection point. The instant a routine is used by two or more different routines then you have found the inflection point.
- Cover the inflection point
Once again this is very different for OO and Procedural.
For OO you have to – in your unit test create an instance of the class under test (where the inflection point lives)
For procedural you have to – in your test – be able to call the function module which is the inflection, or the FORM routine which is the infection point.
All of these have “dependencies” which have to be broken i.e. objects or variable values that the routine “depends on” i.e. presumes exist and presumes that are in a certain state. That is poison to a unit test; you need to find a way to run the routine with all these dependencies in a state where they do not influence the test one way or the other.
This is fairly easy for OO programming, more difficult for function modules, and can be the end of the universe for a FORM routine in a traditional ABAP program which is crawling with global variables.
- Break external dependencies (OO)
This is when a class takes in one or more other classes as IMPORTING parameters during its construction. In ABAP this is best handled by using the “mock” framework to pass in a dummy instance of the needed class which can do the bare minimum, preferably nothing, needed to get the class under test behaving uniformly.
If the class to be passed in needs objects of its own (importing parameters) to be created then that can involve a bit for work.
- Break internal dependencies (OO)
This is when a class does one or more CREATE OBJECT calls during the construction, or worse, during a method. There is no way around this other than to change the class under test such that every time this is encountered you have an importing parameter of the class type and do the following:-
IF io_alien IS SUPPLIED.
Mo_alien = io_alien.
CREATE OBJECT mo_alien.
If the ALIEN class needs objects of its own (importing parameters) to be created you would have to pass the other objects in as well. The constructor would probably be the best place.
As an aside, I have found that this sort of design – passing objects in as optional parameters as opposed to always creating new ones – can sometimes yield very large performance improvements due to the lack of the need for executing lots of complex constructor code repeatedly.
- Break Procedural Dependencies
The best way to make procedural code testable is to convert it into OO code. However, that is easier said than done, with ten billion lines of procedural code and DYNPRO screens all over the shop, and global variables all over your head like a bucket of sick.
At one stage (2001) SAP were considering writing an automated utility to convert all ABAP code into Java. I think they were somewhat optimistic. The concepts are just too different – especially back then before various Java type constructs were introduced to ABAP.
In a sprawling mess of such a program the initial point would be to take whatever FORM routine you have to change and replace the global variables with USING/CHANGING parameters. This will bloat the interface – which could be a clue the routine is doing far too much. This is horrible in that you may find yourself having to change interfaces all up the call stack to move the global variable down (the so called “Tramp Data” effect) but at the very minimum afterwards at least you know where the value came from rather than it popping up like magic from who knows where.
In an SAP application that uses the GUI as opposed to the web you have to use SCREENS and there global variables are mandatory. In Java world you have a framework called SWING which is lovely and OO but in ABAP, no, the only OO user interface technologies use the web. When they do, hooray from a testing perspective but there are so many existing applications using the SAP GUI that are not going anywhere for a long, long, time.
SAP has a clear answer to this. Encapsulate the screen in a function module, which does nothing except accept the data before presenting the screen to the user and then return the user input. Then wrap that module in a class. This is (a) horribly over-complicated and (b) means you cannot use all the inbuilt SAP DYNPRO technologies like ON-CHAIN-INPUT but it does mean you can do automated tests on simulated user inputs.
This is also a gigantic mindset change, and not a trivial exercise for existing programs either where UI, business logic and database access are all welded together, which was the original SAP preferred design. The question has to be, and to me this is a rhetorical question “Is the effort of enabling tests, to make the program stable, worth the transformation effort?”
I have found that DYNPRO programs are the most fragile things I have ever encountered with the smallest change breaking something somewhere every single time. In short, I have never ever encountered something more in need of tests.
- Write tests
By this point, you should be able to use the ABAP Unit framework to create one or more tests to validate the existing behaviour. For current purposes this behaviour is correct, even if the program has been doing something obviously stupid for years.
If you cannot create such a test, then the dependency breaking is not complete.
This step is complete once the test runs the existing code with no changes, and gets all green lights. Not that you actually get green lights in SAP, just a total absence of any feedback whatsoever apart from a tiny “all is well” at the bottom of the screen.
- Make Changes
Now you have a baseline, make your change. You will know instantly if you have broken anything beyond the “infection point”, as you will get red lights when you run the unit tests.
Keep going till you get all green lights.
- Refactor the Covered Code
“First make it work, then make it good”.
Now you are at the stage when you know if you make any sort of change to the routine you are fixing/enhancing whether that change will destroy the universe as we know it.
That means you can make changes to this piece of code with impunity. So now you can do the “refactoring” to make this small routine at least as perfect as it can be e.g. make the variable names meaningful, split it into small routines that do only one thing and so forth.
For procedural ABAP this is the time to see if you can turn the whole thing into a method of a class.
The first time you do this it is going to look ridiculous. I know, I have done this, and a new programmer comes on board and asks why have I got a ten billion line procedural program with a “UI Class” that calls one screen, and a “Business Logic Class” that does about five lines of code, and a “Database Access Class” which does one database read? The point is at least I can test one tiny part of the program, and that part will never break every again. Unless all my colleagues have no idea what unit testing is, or what it is for, or why I have done such a seemingly crazy thing.
The Real Story
Figure 4 – The Real Story
As might be imagined, all this theory is just a load of hot air, if I have not actually tried to do anything of the sort myself. Right at the start of this blog I mentioned I have been at this for about five years now. No-one can say it is easy.
In the following blogs – Feb 017 -I describe the start of the process- for a seemingly innocuous read only report program, and how to bring it back to life.
The program in the blogs above was a read-only report. My main focus – for the five years I just mentioned – has been an interactive ALV report which is the most horrible thing in the history of the universe and which is, of course, business critical. So I should just leave it alone, but no, whenever I have to make a change – which is all the time – I have been doing little pieces of “Boy Scout” surgery. There is still a very long way to go, but in any event I am going to present a little summary of the very slow steps I have taken thus far.
What do you know of the 39 Steps?
- This program has so many global variables it is not funny. A value of a global variable gets set and then read back in 20 subroutines time. Naturally the chance of it getting changed in the interim is very high indeed, and you will never know just where.
- So, as a first step whenever the code I am changing references a global variable. I add some parameters to the routine i.e. instead of changing or reading a global variable within the routine, have that variable as a parameter. That way there are no longer any references to the global variable directly within the routine. To start off with at the next highest level in the call stack you are of course passing the global variable into the routine. Having routines with huge signatures is really bad when it comes to readability, but as a first step at least the journey of a global variable is traced along its long journey.
- That also makes the routine re-usable- in particular you can now extract it to become a method in a class. At the start that is of no benefit in and off itself, but it is a stepping stone on the road to greater things.
- With DYNPRO screens you are a bit stuck. They have to use global variables. At the very minimum I put the call to the screen in its own routine (hopefully a method) and then set the global variables the screen uses just before the CALL SCREEN to make it obvious what gets passed in, and read them all back just afterwards. The incoming variables and outgoing variables are then parameters of the wrapping routine.
- Remove Header Lines. What has that got to do with anything I hear you say? Apart from getting rid of an “obsolete” technique, you get a minor performance benefit when looping through the table using a field symbol. Much more importantly the original program depended heavily on the header line getting filled early on after the user command was processed and then staying untouched through a vast series of subroutine calls. That assumption was not always correct.
- Rename Obscure Variables / FORM Names. There were a bucket load of routine and variable names where there was no way in the world you could tell what they did from looking at the name. Sometimes a name reflected the original use of the variable or routine, and then the program changed at some point so the routine did something utterly different yet the name stayed the same. This is a very non-contentious change, a find/replace and the chance of any functionality changing is low – unless of course you change the name of a global variable, only to find it is used on a DYNPRO screen somewhere. Hopefully the prior step of explicitly listing what goes into and out of each screen should help prevent this problem.
- In preparation for unit testing, in the code that I am changing whenever I encounter an external dependency – a database read, a call to print something, an interface to an external device – that gets put into its own class e.g. database access class, printer class. You cannot test business logic if it is actually going to the database or tries to print something out or what have you.
- The next step is to create a test class. At the start you may only have one test method – to test the exact thing you are changing. In addition it will probably take a gigantic amount of refactoring (i.e. moving database reads into a database access class) just to get that one test working. The first time is the worst – in fact since what is being tested is usually called a CUT (Class Under Test) it could be said that the “first CUT is the deepest”.
- After five years I had enough methods in my local database access class that I decided the time was right to change it into a global class. That is actually really easy – in SE24 you take the menu path “Object Type -> Import -> Local Classes in Program” and efore you can say Jack Robinson your local class is a global class. That will also tell you in a hurry if your local class was doing naughty things like using global variables or making calls to FORM routines.
- No all the database read methods are in a global class it is time to have a second crack at giving them proper names. At the start they had a wide spread of names – all the routines read back data but the routine names started with “read” or “get” or “set” or “init” or “populate” or “write” or about ten zillion other variations. I renamed all non-functional read methods to all say “READ” due to the CRUD naming convention. For a functional method a descriptive name is better as the code would say SELECTED_MONSTER->PUT_HAT_ON( MO_PL->MONSTERS_CURRENT_HEAD( ) ).
- Once the naming is consistent then this helps spot duplicates. I did in fact mind multiple methods doing the exact same database read, but the routines had different names. At some point in the past a programmer had to make change, looked for an existing routine that read the data and could not find it due to the chaotic naming convention, concluded there was no existing routine, and proceeded to create another one, giving it an even more obscure name, Then the exact same thing happened two years later, then again the next year and so on. Trying to give all those routines a logical name highlight this, so you can remove the duplicates.
- Staying on the subject of naming “IS_A” is OK for functional methods, as then it sounds like English i.e. IF MO_PL->IS_A_VAMPIRE( SELECTED_MONSTER) THEN even if that sounds a bit like Yoda writing the code. Indeed for a function method a name like GOODS_ISSUE_STATUS is OK for the same reason i.e. IF MO_PL->GOODS_ISSUE_STATUS_OF( DELIVERY ) = MC_ISSUED THEN….
- Create Model class – at first this just has assorted data definitions (TYPES and GCV and constants) that the PL needs. In other words at the start of the process the TOP INCLUDE had assorted global types and non DDIC table definitions. Rather than having Z table types which would not be sued outside this application, these definitions can becomes public attributes of a model class. I like model classes – I think they are great. They have class, and they are the very model of a modern major general.
- Here is a tricky one – it seems that I need a “selection option” class that would read the selection options off the screen at start-up and get passed into the PL when it is created. I would ask if anyone in the world has found a good way to marry selection-options with OO classes. You could pass the selection options into the constructor of such a class one by one, or you could pass in a structure. I would ask – why is a structure better than a big signature? You still have to move the values one by one, into the structure as opposed to the constructor.
- At this stage some routines have so much conditional logic they are crying out for polymorphism, but let us hold back for the moment, as that is enough changes for the time being.
The Futures So Scary, I’ve Got to Wear Shades
At this point, even after five years’ worth of changes, things are a lot better than before, but I still feel like I have only changed the tip of the iceberg. There are some unit tests, and a global database access class, and a lot more OO code than before, and the naming is better, but the beast is still 90% procedural and still crawling with global variables.
Figure 5 – Started so I will Finish
Still there is no turning back now. Hopefully with each change I have made the program has become more stable (due to unit tests) and more maintainable (due to clearer naming and less global variables).
The real challenge is going to come in very short order due to the fact that the scenario I have been preparing for – this monstrosity has to exist in every country and behave differently in each – has now moved from being a remote possibility to being an actual requirement and is now coming down at me like an express train.
So this journey to the centre of the earth is not over by a long chalk yet, in fact I have not even got as far as Jimi Hendrix has. So no doubt in another year I will be posting an update to this blog, with some more lessons learned about taming “big balls of mud”.
As always I would love to hear other people’s experiences in this area.