topic Re: Improve ABAP Code in Application Development and Automation Discussions

Improve ABAP Code

former_member799034 — Mon, 25 Apr 2022 10:22:04 GMT

Hi,

Experts, Is there any better way to do the following coding? The main purpose of the following code is to distribute total stock to different periods from front to back, Every period has already calculated the total receipt quantity.

thanks!

*Define

*-----------------------------------------------------------------
DEFINE MOVE_SUB.
d_menge_i = d_menge_i_sub. clear d_menge_i_sub.
END-OF-DEFINITION.

DEFINE SUB_M_D.
d_menge_i_sub = d_menge_i - &1.
END-OF-DEFINITION.

DEFINE MOVE_M_D.
&1 = d_menge_i.

END-OF-DEFINITION.

D_MENGE_I = D_MENGE_I_E - D_MENGE_I_1. " total stock - first month receipt quanity

IF D_MENGE_I <= 0. "1M
IT_DATA-MENGE_1 = D_MENGE_I_E.
ELSE.
IT_DATA-MENGE_1 = D_MENGE_I_1.
SUB_M_D D_MENGE_I_2.
IF D_MENGE_I_SUB <= 0. "1-2M
MOVE_M_D IT_DATA-MENGE_2.
ELSE.
IT_DATA-MENGE_2 = D_MENGE_I_2.
MOVE_SUB.
SUB_M_D D_MENGE_I_3.
IF D_MENGE_I_SUB <= 0. "2-3M
MOVE_M_D IT_DATA-MENGE_3.
ELSE.
IT_DATA-MENGE_3 = D_MENGE_I_3.
MOVE_SUB.
SUB_M_D D_MENGE_I_4.
IF D_MENGE_I_SUB <= 0. "4-6M
MOVE_M_D IT_DATA-MENGE_4.
ELSE.
IT_DATA-MENGE_4 = D_MENGE_I_4.
MOVE_SUB.
SUB_M_D D_MENGE_I_51.
IF D_MENGE_I_SUB <= 0. "7M
MOVE_M_D IT_DATA-MENGE_51.
ELSE.
IT_DATA-MENGE_51 = D_MENGE_I_51.
MOVE_SUB.
SUB_M_D D_MENGE_I_52.
IF D_MENGE_I_SUB <= 0. "8M
MOVE_M_D IT_DATA-MENGE_52.
ELSE.
IT_DATA-MENGE_52 = D_MENGE_I_52.
MOVE_SUB.
SUB_M_D D_MENGE_I_53.
IF D_MENGE_I_SUB <= 0. "9M
MOVE_M_D IT_DATA-MENGE_53.
ELSE.
IT_DATA-MENGE_53 = D_MENGE_I_53.
MOVE_SUB.
SUB_M_D D_MENGE_I_54.
IF D_MENGE_I_SUB <= 0. "10M
MOVE_M_D IT_DATA-MENGE_54.
ELSE.
IT_DATA-MENGE_54 = D_MENGE_I_54.
MOVE_SUB.
SUB_M_D D_MENGE_I_55.
IF D_MENGE_I_SUB <= 0. "11M
MOVE_M_D IT_DATA-MENGE_55.
ELSE.
IT_DATA-MENGE_55 = D_MENGE_I_55.
MOVE_SUB.
SUB_M_D D_MENGE_I_56.
IF D_MENGE_I_SUB <= 0. "12M
MOVE_M_D IT_DATA-MENGE_56.
ELSE.
IT_DATA-MENGE_56 = D_MENGE_I_56.
MOVE_SUB.
SUB_M_D D_MENGE_I_6.
IF D_MENGE_I_SUB <= 0. "1-2Y
MOVE_M_D IT_DATA-MENGE_6.
ELSE.
IT_DATA-MENGE_6 = D_MENGE_I_6.
MOVE_SUB.
SUB_M_D D_MENGE_I_7.
IF D_MENGE_I_SUB <= 0. "2-3Y
MOVE_M_D IT_DATA-MENGE_7.
ELSE. ">3Y
IT_DATA-MENGE_7 = D_MENGE_I_7.
IT_DATA-MENGE_8 = D_MENGE_I_SUB.
ENDIF.
ENDIF.
ENDIF.
ENDIF.
ENDIF.
ENDIF.
ENDIF.
ENDIF.
ENDIF.
ENDIF.
ENDIF.
ENDIF.

APPEND IT_DATA.

Re: Improve ABAP Code

Jeansy — Mon, 25 Apr 2022 10:31:17 GMT

Hi,

please use the CODE-function in your posting, that makes code much better readable.

I already see the definition of macros. I would not use macros anymore as they are deprecated.

Kind regards
Jens

Re: Improve ABAP Code

former_member799034 — Mon, 25 Apr 2022 10:40:58 GMT


*Define
*-----------------------------------------------------------------
DEFINE MOVE_SUB.
  d_menge_i = d_menge_i_sub. clear d_menge_i_sub.
END-OF-DEFINITION.

DEFINE SUB_M_D.
  d_menge_i_sub = d_menge_i - &1.
END-OF-DEFINITION.

DEFINE MOVE_M_D.
  &1 = d_menge_i.
END-OF-DEFINITION.

      D_MENGE_I = D_MENGE_I_E - D_MENGE_I_1.  "Total stock - first month receipt quanity

      IF D_MENGE_I <= 0.                            "1M
        IT_DATA-MENGE_1 = D_MENGE_I_E.
      ELSE.
        IT_DATA-MENGE_1 = D_MENGE_I_1.

        SUB_M_D D_MENGE_I_2.
        IF D_MENGE_I_SUB <= 0.                          "1-2M
          MOVE_M_D IT_DATA-MENGE_2.
        ELSE.
          IT_DATA-MENGE_2 = D_MENGE_I_2.
          MOVE_SUB.
          SUB_M_D D_MENGE_I_3.
          IF D_MENGE_I_SUB <= 0.                        "2-3M
            MOVE_M_D IT_DATA-MENGE_3.
          ELSE.
            IT_DATA-MENGE_3 = D_MENGE_I_3.
            MOVE_SUB.
            SUB_M_D D_MENGE_I_4.
            IF D_MENGE_I_SUB <= 0.                      "4-6M
              MOVE_M_D IT_DATA-MENGE_4.
            ELSE.
              IT_DATA-MENGE_4 = D_MENGE_I_4.
              MOVE_SUB.
              SUB_M_D D_MENGE_I_51.
              IF D_MENGE_I_SUB <= 0.                      "7M
                MOVE_M_D IT_DATA-MENGE_51.
              ELSE.
                IT_DATA-MENGE_51 = D_MENGE_I_51.
                MOVE_SUB.
                SUB_M_D D_MENGE_I_52.
                IF D_MENGE_I_SUB <= 0.                      "8M
                  MOVE_M_D IT_DATA-MENGE_52.
                ELSE.
                  IT_DATA-MENGE_52 = D_MENGE_I_52.
                  MOVE_SUB.
                  SUB_M_D D_MENGE_I_53.
                  IF D_MENGE_I_SUB <= 0.                      "9M
                    MOVE_M_D IT_DATA-MENGE_53.
                  ELSE.
                    IT_DATA-MENGE_53 = D_MENGE_I_53.
                    MOVE_SUB.
                    SUB_M_D D_MENGE_I_54.
                    IF D_MENGE_I_SUB <= 0.                      "10M
                      MOVE_M_D IT_DATA-MENGE_54.
                    ELSE.
                      IT_DATA-MENGE_54 = D_MENGE_I_54.
                      MOVE_SUB.
                      SUB_M_D D_MENGE_I_55.
                      IF D_MENGE_I_SUB <= 0.                      "11M
                        MOVE_M_D IT_DATA-MENGE_55.
                      ELSE.
                        IT_DATA-MENGE_55 = D_MENGE_I_55.
                        MOVE_SUB.
                        SUB_M_D D_MENGE_I_56.
                        IF D_MENGE_I_SUB <= 0.                      "12M
                          MOVE_M_D IT_DATA-MENGE_56.
                        ELSE.
                          IT_DATA-MENGE_56 = D_MENGE_I_56.
                          MOVE_SUB.
                          SUB_M_D D_MENGE_I_6.
*                          IF D_MENGE_I_SUB <= 0 .                 "7-12M
*                            MOVE_M_D IT_DATA-MENGE_5.
*                          ELSE.
*                            IT_DATA-MENGE_5 = D_MENGE_I_5.
*                            MOVE_SUB.
*                            SUB_M_D D_MENGE_I_6.
                          IF D_MENGE_I_SUB <= 0.                  "1-2Y
                            MOVE_M_D IT_DATA-MENGE_6.
                          ELSE.
                            IT_DATA-MENGE_6 = D_MENGE_I_6.
                            MOVE_SUB.
                            SUB_M_D D_MENGE_I_7.
                            IF D_MENGE_I_SUB <= 0.                "2-3Y
                              MOVE_M_D IT_DATA-MENGE_7.
                            ELSE.                                 ">3Y
                              IT_DATA-MENGE_7 = D_MENGE_I_7.
                              IT_DATA-MENGE_8 = D_MENGE_I_SUB.
                            ENDIF.
                          ENDIF.
                        ENDIF.
                      ENDIF.
*                      ENDIF.
                    ENDIF.
                  ENDIF.
                ENDIF.
              ENDIF.
            ENDIF.
          ENDIF.
        ENDIF.
      ENDIF.

      IT_DATA-MENGE_5 = IT_DATA-MENGE_51 + IT_DATA-MENGE_52 +
                        IT_DATA-MENGE_53 + IT_DATA-MENGE_54 +
                        IT_DATA-MENGE_55 + IT_DATA-MENGE_56.
      APPEND IT_DATA.

Re: Improve ABAP Code

FredericGirod — Mon, 25 Apr 2022 11:16:38 GMT

as jzaehringer, macro is deprecated

use Oo, with a simple recursive method

You should also use ABAP Unit to test your code

Re: Improve ABAP Code

former_member799034 — Mon, 25 Apr 2022 12:01:22 GMT

Well, these codes may have a long history, It's done ten years ago, Recently my customer has some new business request， So when I start to work and I look at this long long code， I thought maybe I can make a little bit improve，make it looks more clear， But I don't want to rewrite all code.

Re: Improve ABAP Code

FredericGirod — Mon, 25 Apr 2022 12:16:15 GMT

https://github.com/SAP/styleguides/blob/main/clean-abap/CleanABAP.md#how-to-refactor-legacy-code

Follow the boy scout rule to your daily work routine: always leave the code you edit a little cleaner than you found it. Don't obsess with this by sinking hours into "cleaning the campsite", just spend a couple of minutes extra and observe how the improvements accumulate over time.

You have no choice, to keep a cleaner place, you have to fully rewrite it.

Re: Improve ABAP Code

matt — Mon, 25 Apr 2022 12:16:39 GMT

william.tse

Before making adding the new logic, get the code into a test harness. I.e. write ABAP Unit Tests and make sure it passes them all.
Replace the macros with calls to methods - make sure all your unit tests pass
Rename the variables into something meaningful d_menge_i_e and d_menge_i_1 are not terribly meaningful
Rewrite the code so that what it does is readily understandable. Run the unit tests again and fix until it passes them all.
Add a new unit test for the new requirement.
Write and fix until the new test is passed by the code.

Bit the bullet. Rewrite the code. It's awful - expensive and shoddy. You've got a ten year technical debt there. If you pay off the debt now, then you'll be able to do the next change far more quickly and easily. Or your successor will.

Re: Improve ABAP Code

FredericGirod — Mon, 25 Apr 2022 12:28:33 GMT

if you could post little bit more of this code, we could help you for ABAP Unit (could scary little bit the first time you see it, but it will give you great Abap power 😉 )

Re: Improve ABAP Code

Sandra_Rossi — Mon, 25 Apr 2022 14:25:31 GMT

I agree with Matthew, the code is awful, I can't understand what it does, use meaningful names, etc. (do what Matthew suggested).

Re: Improve ABAP Code

former_member799034 — Tue, 26 Apr 2022 01:32:26 GMT

OK, I will try it, thanks for your advice