Personal Insights
Refactoring an old report – some strange code explained
Not sure how to start this, so I’ll jump right in:
in the past I was tasked to re-factor an old (first line coded > 15 years ago) but still relevant report. Recently I came back to it an found code like that:
DATA: l_atp_stock_fka_lagerbestand LIKE lagerbestand.
l_atp_stock_fka_lagerbestand = get_verfuegbar_atp(
i_werks = l_marc-werks
i_matnr = l_marc-matnr ).
lagerbestand = l_atp_stock_fka_lagerbestand.A new variable B is defined LIKE an existing variable A.
Then B is filled by a method call.
Then A is filled from B.
(Not in picture: there is no further use of B!).
Looking through version history (and my mind – after a short irritation, I was pretty sure why I did this!) shows why:
At one point in time, the code looked like this:
PERFORM get_verfuegbar USING l_marc-werks l_marc-matnr
CHANGING lagerbestand.
DATA: l_atp_stock_fka_lagerbestand LIKE lagerbestand.
l_atp_stock_fka_lagerbestand = get_verfuegbar_atp(
i_werks = l_marc-werks
i_matnr = l_marc-matnr ).
lagerbestand = l_atp_stock_fka_lagerbestand.Explanation: Being very old, this report used lots of FORM / PERFORM.
Refactoring it, I changed that to using method calls wherever possible.
Of course, I wanted to make sure the new code would produce the same result as the old one.
This is what that construct allowed me to do:
The new code would always “win” as it would overwrite what was calculated in the old code.
But I also had the possibility to set a breakpoint and compare those results in the debugger.
(I think I didn’t, but I could even have added an ASSERT-Statement).
So that’s the story of this little piece of code.
I now change it to simply look like this:
lagerbestand = get_verfuegbar_atp(
i_werks = l_marc-werks
i_matnr = l_marc-matnr ).-> so that story would be hidden deep in version history had I not shared it here.
What do you think?
Had you similar encounters?
Would you have chosen a different way?
best
Joachim
Hello Joachim,
What do you think? -> well done! the code has certainly improvedĀ
Had you similar encounters? -> yes, of course. I like to refactor with the aim of making the code simpler or making my understanding of the problem explicit. The first version is First make it work, cf the Joy Of Coding, then make it right. At the end of the day, the question is always: is this change really an improvement worth the risk of changing productive code?
Would you have chosen a different way? -> my question would be always be: how can I do this safely?
I see your approach as safe, as you could test by calling the legacy routine.
Today I would first write an ABAP unit test (a so called characterization test to document the current system behavior) before changing the code. If I have a good test coverage, I will change code to my heart content.
In other cases, I will go on without automated tests if the code will be going through a QA process where the specification is validated by someone who understands the problem domain well enough to perform integration tests.
And there is Test Driven Development, as recommended in the openSAP course: while working on legacy code we should gradually extract functionality to an island of happyness were the new code is completely covered by tests.
best regards,
JNN
I agree with your statements!
Thanks very much for you feedback, very helpful!
(I only see it now, seems like I didn't get a notification...)
I often work on enhancements to older, procedural applications and have the same challenges as everyone. I often use a "bridge" from the old procedural source code to object-oriented source code (Isle of Happiness).
However, it has been shown that overarching concepts suffer as a result. Sometimes they cannot be realized at all because you work selectively. It feels like there are applications that at some point cannot be maintained with any method. Actually, you would have to completely rewrite it and put a lot of thought into the architecture in advance. But only very few do that š