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:

* 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.

Conclusion

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!