Skip to Content
Technical Articles
Author's profile photo Suhas Saha

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)
Running 20210207080000 20210207083500
Swimming 20210207090000 20210207091500
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.

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!

Assigned Tags

      16 Comments
      You must be Logged on to comment or reply to a post.
      Author's profile photo Sandra Rossi
      Sandra Rossi

      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:

      R_RESULT = reduce #( 
                   init RES type ZIF_ACTIVITY_TRACKER=>TY_DETAILED_ACTIVITY_TIMES
      
                   for <A> in _ACTIVITIES
      
                   next
      
                    " Time spent on a completed activity
                    RES-COMPLETED += 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
                    RES-RUNNING += cond #(
                          when <A>-STOP_TIME is initial
                            then CL_ABAP_TSTMP=>SUBTRACT(
                                   TSTMP1 = L_INTERIM_STOP
                                   TSTMP2 = <A>-START_TIME ) ) ).
      Author's profile photo Suhas Saha
      Suhas Saha
      Blog Post Author

      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?

      Author's profile photo Sandra Rossi
      Sandra Rossi

      Yes with 7.52 in an executable program (can't use +=) :

      REPORT.
      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.
      TYPES:
        BEGIN OF ty_detailed_activity_times,
          completed TYPE i,
          running   TYPE i,
        END OF ty_detailed_activity_times.
      DATA r_result TYPE ty_detailed_activity_times.
      
      r_result = REDUCE #(
                   INIT res TYPE ty_detailed_activity_times
      
                   FOR <a> IN _activities
      
                   NEXT
      
                    " Time spent on a completed activity
                    res-completed = res-completed + 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
                    res-running = res-running + COND #(
                          WHEN <a>-stop_time IS INITIAL
                            THEN cl_abap_tstmp=>subtract(
                                   tstmp1 = 1
                                   tstmp2 = <a>-start_time ) ) ).
      
      Author's profile photo Suhas Saha
      Suhas Saha
      Blog Post Author

      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?

      Author's profile photo Sandra Rossi
      Sandra Rossi

      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:

      r_result = VALUE #( ).
      LOOP AT ...
      Author's profile photo Sandra Rossi
      Sandra Rossi

      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:

      _activities = VALUE #(
      ( id = `Running ` start_time = '20210207080000' stop_time = '20210207083500' )
      ( id = `Swimming` start_time = '20210207090000' stop_time = '20210207091500' )
      ( id = `Cycling ` start_time = '20210207140000' stop_time = '' )
      ).
      
      cl_abap_tstmp=>systemtstmp_syst2utc( EXPORTING syst_date = sy-datum syst_time = sy-uzeit
        IMPORTING utc_tstmp = data(now) ).
      
      r_result = REDUCE #(
                   INIT res TYPE ty_detailed_activity_times
      
                   FOR <a> IN _activities
      
                   NEXT res = cond #( when <a>-stop_time IS NOT INITIAL then
      
                              " Time spent on a completed activity
                              VALUE #( base RES
                              completed = res-completed + cl_abap_tstmp=>subtract(
                                          tstmp1 = <a>-stop_time
                                          tstmp2 = <a>-start_time ) )
                          else
      
                              " Time spent on a running activity
                              VALUE #( base RES
                              running   = res-running + cl_abap_tstmp=>subtract(
                                          tstmp1 = now
                                          tstmp2 = <a>-start_time ) ) ) ).
      
      Author's profile photo Suhas Saha
      Suhas Saha
      Blog Post Author

      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.

      Author's profile photo Edo von Glan
      Edo von Glan

      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.

      Author's profile photo Shai Sinai
      Shai Sinai

      An interesting topic.

      Maybe the functional programming representative, Sougata Chatterjee, may add another point-of-view here :).

      Author's profile photo Sougata Chatterjee
      Sougata Chatterjee

      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 ๐Ÿ˜Š

      Author's profile photo Patrick Van Nierop
      Patrick Van Nierop

      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.

      Author's profile photo Sougata Chatterjee
      Sougata Chatterjee

      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?

      Author's profile photo Patrick Van Nierop
      Patrick Van Nierop

      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 :).

      Author's profile photo Sougata Chatterjee
      Sougata Chatterjee

      You actually can debug a REDUCE statement ๐Ÿ™‚

      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.

      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?

      Author's profile photo Suhas Saha
      Suhas Saha
      Blog Post Author

      Don't think you can debug a REDUCE statement ๐Ÿ˜‰

      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 ๐Ÿ˜’

      Author's profile photo Patrick Van Nierop
      Patrick Van Nierop

      Thanks! I learned something new today ๐Ÿ˜‰