Let’s not REDUCE the readability of the code
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!