Let’s not REDUCE the readability of the code
REDUCE has been around since ABAP Release 740. There have been some useful blogs on it, especially this one from Horst Keller.
I love REDUCE and never miss a chance of using it. The typical use cases for me are summing up or concatenating the contents of a field of an internal table.
However, IMHO, developers are still skeptical on its usage. I often get the feedback that REDUCE is difficult to understand and does not represent “clean” code.
This made me reflect on my usage of REDUCE. Here are my thoughts…
Let’s assume we are developing an activity tracker app which stores the details of a user’s activities as follows:
|Activity ID (String)||Start Time (Timestamp)||Stop Time (Timestamp)|
|Cycling||20210207140000||<initial Value> *|
* Stop Time = <initial Value> indicates the activity is not yet complete.
The app provides the following data to the user on demand:
- Total active time
- Detailed activity times viz., time of completed and not complete (read: running) activities
To realize this, the following interface was defined
interface ZIF_ACTIVITY_TRACKER public . types: begin of TY_DETAILED_ACTIVITY_TIMES, COMPLETED type I, RUNNING type I, end of TY_DETAILED_ACTIVITY_TIMES. methods: " ... Start & stop activity methods are obvioulsy there !!! GET_TOTAL_ACTIVE_TIME returning value(R_RESULT) type I, GET_DETAILED_ACTIVITY_TIMES returning value(R_RESULT) type TY_DETAILED_ACTIVITY_TIMES. endinterface.
The class implementing the interface stores the activities in a table attribute _ACTIVITIES as defined above
types: begin of _TY_ACTIVITY, ID type STRING, START_TIME type TIMESTAMP, STOP_TIME type TIMESTAMP, end of _TY_ACTIVITY. data _ACTIVITIES type hashed table of _TY_ACTIVITY with unique key ID.
Get the total time the user was active
My first approach was to go all-in with functions. ‘Coz they are cool, aren’t they? 😎
method ZIF_ACTIVITY_TRACKER~GET_TOTAL_ACTIVE_TIME. get time stamp field data(L_INTERIM_STOP). R_RESULT = reduce #( init T = 0 for <A> in _ACTIVITIES next T += CL_ABAP_TSTMP=>SUBTRACT( TSTMP1 = cond #( when <A>-STOP_TIME is initial then L_INTERIM_STOP else <A>-STOP_TIME ) TSTMP2 = <A>-START_TIME ) ). endmethod.
Well, looking at the code again, it did seem a bit cryptic (read: even to me). Hey, what’s the deal with the COND? 🤷♂️
Maybe if we extracted the COND expression into an auxiliary variable, it could help cleanup the call of CL_ABAP_TSTMP=>SUBTRACT( ). This was what the code looked like after this “cosmetic” refactoring
R_RESULT = reduce #( init T = 0 for <A> in _ACTIVITIES let STOP = cond #( when <A>-STOP_TIME is initial then L_INTERIM_STOP else <A>-STOP_TIME ) in next T += CL_ABAP_TSTMP=>SUBTRACT( TSTMP1 = STOP TSTMP2 = <A>-START_TIME ) ).
Now it is clear to the “human” reader that when STOP_TIME is initial, we use the intermediate timestamp (L_INTERIM_STOP) to calculate the time spent on the (running) activity.
N.B.: When using functions, i tend to add a single space between operators so as not to strain the reader’s eyes. This is in line with the clean code guideline.
Get the detailed activity times
In this case we split the total time spent into the time spent on completed & not completed activities.
The first approach was to have two REDUCE’s.
method ZIF_ACTIVITY_TRACKER~GET_DETAILED_ACTIVITY_TIMES. get time stamp field data(L_INTERIM_STOP). R_RESULT-COMPLETED = reduce #( init T = 0 for <A> in _ACTIVITIES where ( STOP_TIME is not initial ) next T += CL_ABAP_TSTMP=>SUBTRACT( TSTMP1 = <A>-STOP_TIME TSTMP2 = <A>-START_TIME ) ). R_RESULT-RUNNING = reduce #( init T = 0 for <A> in _ACTIVITIES where ( STOP_TIME is initial ) next T += CL_ABAP_TSTMP=>SUBTRACT( TSTMP1 = L_INTERIM_STOP TSTMP2 = <A>-START_TIME ) ). endmethod.
My unit test(s) had passed, but something was still nagging me. My thumb rule is not to have multiple iterations on the same internal table in a single procedure. The code above is iterating on the table _ACTIVITIES twice! Let’s refactor this code.
R_RESULT = reduce #( init RES type ZIF_ACTIVITY_TRACKER=>TY_DETAILED_ACTIVITY_TIMES for <A> in _ACTIVITIES let " Time spent on a completed activity TC = cond #( when <A>-STOP_TIME is not initial then CL_ABAP_TSTMP=>SUBTRACT( TSTMP1 = <A>-STOP_TIME TSTMP2 = <A>-START_TIME ) ) " Time spent on a running activity TR = cond #( when <A>-STOP_TIME is initial then CL_ABAP_TSTMP=>SUBTRACT( TSTMP1 = L_INTERIM_STOP TSTMP2 = <A>-START_TIME ) ) in next RES-COMPLETED += TC RES-RUNNING += TR ).
* See the code snippet from Sandra Rossi how to achieve this without the auxiliary variables
I am reducing my internal table _ACTIVITIES into the target structure R_RESULT whose fields are populated conditionally based on the field _ACTIVITIES-STOP_TIME.
I could achieve it using REDUCE function, but does it really make my code clean & readable? I had my doubts and hence i decided to refactor the code using the classic LOOP.
get time stamp field data(L_INTERIM_STOP). loop at _ACTIVITIES assigning field-symbol(<ACTIVITY>). if <ACTIVITY>-STOP_TIME is not initial. R_RESULT-COMPLETED += CL_ABAP_TSTMP=>SUBTRACT( TSTMP1 = <ACTIVITY>-STOP_TIME TSTMP2 = <ACTIVITY>-START_TIME ). else. R_RESULT-RUNNING += CL_ABAP_TSTMP=>SUBTRACT( TSTMP1 = L_INTERIM_STOP TSTMP2 = <ACTIVITY>-START_TIME ). endif. endloop.
IMHO the last option provides the most clear and concise picture of the logic. It is the most clean implementation too, unless you are OK with looping on the same table twice.
I want to highlight that using functions always can lead to cryptic, unreadable code. Too many nested functions, auxiliary variables tend to bloat the function unnecessarily.
Functions are powerful, use them wisely. As the wise Uncle Ben told Spidey, “With great power comes great responsibility”. Code responsibly!
Here, you are developing one example, but the issue is with any code. How to know what is the average level of developers at a company? What is "too complex"? If the peer review leads to an interrogation and the negotiation fails, then maybe 2 other fellows (named by the 2 initial developers) could vote on the concerned problem.
I don't know why, but the auxiliary variables of REDUCE bugs me. This code without them looks identical in complexity to the LOOP AT:
How to know what is the average level of developers at a company? What is "too complex"?
Complexity is relative, IMO. My observation is that developers are not too comfortable using REDUCE function. My idea was to make the REDUCE a bit more palatable to the readers.
I tried without the LET variables too, but they produce syntax errors. Fyi, i am on the latest ABAP release.
Does it work for you?
Yes with 7.52 in an executable program (can't use +=) :
Got it. Since using += , i forgot x = x + 1 still works 😉
However, the REDUCE in the case of GET_DETAILED_ACTIVITY_TIMES is a bit cryptic. Wouldn't you say the LOOP is easier to comprehend & is concise?
I agree that LOOP is more clear
I like very much the constructor expressions because it's always obvious that the resulting variable is initial at the beginning, and that only this variable is changed (not talking about method calls and attributes...)
If we could add that before LOOP AT, that would be perfect, but since it's known that R_RESULT is a return parameter and so is initial, then it's okay:
Your blog post is great because many people want desperately to convert their code into constructor expressions, without explaining the reason, even if their current code is clear. Maybe they think that will improve the performance. That won't, and it can even reduce the performance.
Another fun way, completely counter-performing and not more clear than LOOP AT:
I love the constructor expressions. Hope people don't get me wrong 🤦♂️
Where did people get the idea that constructor expressions are faster than their verbose counterparts? 😵
Through my blog i just want to iterate the fact that we must not make the code unreadable just for the sake of being cool and modern.
I also think that it is good fun to try implementing logic using the constructor expressions etc.
At the same time, I think one should step back at the end, and compare it to an "old-fashioned" solution. In some cases, I find that the old-fashioned solution is not much longer but much better understandable by an average ABAP programmer.
An interesting topic.
Maybe the functional programming representative, Sougata Chatterjee, may add another point-of-view here :).
I think we should just move on with the times because ‘the times are a changing’ and “change” is the only constant in the Universe. If you can’t read it then make the effort to learn it - keep up and stop whinging 😊
I'm always willing to accept change, just as long as it isn't change for the sake of change. If that change will result in a better way of doing things, then I'm all for it.
(Stole this one from James Van Fleet)
And as a side note, referring to Clean code, it's about having a code base, which is readable and maintainable being essential for sustainable development. So, new syntax is a tool for me, not a goal.
One however could argue that the attitude “change for the sake of change” stems from the fact, in this case, of system backward compatibility. Had it not been the case, say for new applications, then the forced change would look and feel quite okay after debugging then reading it a few times wouldn’t it?
Don't think you can debug a REDUCE statement ;).
But don't get me wrong, personally enjoy getting to know new syntax, and I try to keep my skills up-to-date. On the other hand, I see no need for a forced change in this domain. I'd rather see people adopt clean code principles personally.
Maintainability, performance, scalability,..etc are domains I like to see developers progress in. In this, I see new syntax as a tool, nothing more, nothing less.
And when working in team, I guess you'll have to take all team members into account. If we produce code that our functional colleagues can't understand, then they'll stop debugging. If that happens, that will mean more work for the development team.
This is of the things that aren't black or white I believe. There is no single truth, but just a load of shades of grey :).
You actually can debug a REDUCE statement
Although they have my sympathy but what do I do as a modern developer? How do I not do functional programming because 1. that's how I do it best and 2. I'm been advised by my architect that very soon these apps I'm building are going to be deployed in the Cloud so they need to be cloud compatible (example scenario).
Isn't it be better if the team spent some time and effort now rather than later to understand these concepts before more and more old syntaxes will be disallowed across platforms?
Yes, we can Adding my 2c to what Sougata Chatterjee has already mentioned
To debug the table expressions viz., FOR & REDUCE, you have to set the "Step Size" of the debugger to "Subcondition/Statement"
Afaik, this feature is still not available in ADT
Thanks! I learned something new today 😉