Application Development Discussions
Join the discussions or start your own on all things application development, including tools and APIs, programming models, and keeping your skills sharp.
cancel
Showing results for 
Search instead for 
Did you mean: 

How can I optimize my code

Q1. In 2019, what are the best practices to follow while coding in ABAP?

Q2. Have a look at my code and let me know, how can I optimize it. Also, please let me know how can I remove "TABLES".


Thanks.

*---------------------------------------------------------------------*
* TABLES *
*---------------------------------------------------------------------*
Tables: MARA, MAKT, MARD, VBAK, VBAP.
*---------------------------------------------------------------------*
* TYPES
*---------------------------------------------------------------------*
Types : Begin Of ITAB1,
 MATNR Type MARA-MATNR,
 ERSDA Type MARA-ERSDA,
 ERNAM Type MARA-ERNAM,
 LAEDA Type MARA-LAEDA,
 MTART Type MARA-MTART,
 MATKL Type MARA-MATKL,
 MEINS Type MARA-MEINS,
End Of ITAB1.
Types: Begin Of ITAB2,
 MATNR Type MAKT-MATNR,
 MAKTG Type MAKT-MAKTG,
End Of ITAB2.
Types: Begin Of ITAB3,
 MATNR Type MARD-MATNR,
 LGORT Type MARD-LGORT,
 WERKS Type MARD-WERKS,
End Of ITAB3.
*
Types: Begin Of ITAB4,
 MATNR Type MARA-MATNR,
 ERSDA Type MARA-ERSDA,
 ERNAM Type MARA-ERNAM,
 LAEDA Type MARA-LAEDA,
 MTART Type MARA-MTART,
 MATKL Type MARA-MATKL,
 MEINS Type MARA-MEINS,
 MAKTG Type MAKT-MAKTG,
 LGORT Type MARD-LGORT,
 WERKS Type MARD-WERKS,
End Of ITAB4.
Types: Begin Of ITAB6,
 VBELN Type VBAK-VBELN,
 ERDAT Type VBAK-ERDAT,
End Of ITAB6.
Types: Begin Of ITAB5,
 VBELN Type VBAP-VBELN,
 POSNR Type VBAP-POSNR,
 MATNR Type VBAP-MATNR,
 MATWA Type VBAP-MATWA,
End Of ITAB5.
Types: Begin Of ITAB9,
 VBELN Type VBAK-VBELN,
 ERDAT Type VBAK-ERDAT,
 POSNR Type VBAP-POSNR,
 MATNR Type VBAP-MATNR,
 MATWA Type VBAP-MATWA,
End Of ITAB9.
*---------------------------------------------------------------------*
* DATA
*---------------------------------------------------------------------*
Data: IT_MARA Type Standard Table Of ITAB1,
 WA_MARA TYPE ITAB1,
 IT_MAKT Type Standard Table Of ITAB2,
 WA_MAKT TYPE ITAB2,
 IT_MARD Type Standard Table Of ITAB3,
 WA_MARD TYPE ITAB3,
 IT_FINAL Type Standard Table Of ITAB4,
 WA_FINAL TYPE ITAB4,
 IT_FCAT Type SLIS_T_FIELDCAT_ALV,
 WA_FCAT Like Line Of IT_FCAT,
 IT_VBAP Type Standard Table Of ITAB5,
 WA_VBAP Type ITAB5,
 IT_VBAK Type Standard Table Of ITAB6,
 WA_VBAK Type ITAB6,
 IT_FINAL1 Type Standard Table Of ITAB9,
 WA_FINAL1 Type ITAB9.
*---------------------------------------------------------------------*
* SELECTION SCREEN *
*---------------------------------------------------------------------*
SELECTION-Screen Begin Of Block B1 With Frame Title TEXT01.
 SELECTION-Screen Skip 1.
 Select-OPTIONS: Date For SY-DATUM Default ' ' To SY-DATUM OBLIGATORY.
 SELECTION-Screen Skip 1.
 SELECTION-Screen Begin Of Block B2 With Frame Title TEXT02.
 SELECTION-Screen Skip 1.
 Parameters: MATRL_R Radiobutton Group G1 USER-COMMAND D1 Default 'X',
 SALES_R Radiobutton Group G1 .
 SELECTION-Screen Skip 1.
 SELECTION-Screen End Of Block B2.
 SELECTION-Screen Begin Of Block B3 With Frame Title TEXT03.
 SELECTION-Screen Skip 1.
 SELECT-OPTIONS: MATNR FOR MARA-MATNR MODIF ID D1 NO INTERVALS.
 SELECT-OPTIONS: VBELN FOR VBAK-VBELN MODIF ID D2 NO INTERVALS.
 SELECTION-Screen Skip 1.
 SELECTION-Screen End Of Block B3.
SELECTION-Screen End Of Block B1.
At SELECTION-Screen Output.
Loop At Screen.
 If Screen-GROUP1 = 'D1'.
 If MATRL_R = 'X'.
 Screen-INVISIBLE = '0'.
 Screen-ACTIVE = '1'.
 Else.
 Screen-INVISIBLE = '1'.
 Screen-ACTIVE = '0'.
 Endif.
 Modify Screen.
 Endif.
 If Screen-GROUP1 = 'D2'.
 If SALES_R = 'X'.
 Screen-INVISIBLE = '0'.
 Screen-ACTIVE = '1'.
 Else.
 Screen-INVISIBLE = '1'.
 Screen-ACTIVE = '0'.
 Endif.
 Modify Screen.
 Endif.
Endloop.
*---------------------------------------------------------------------*
* INITIALIZATION *
*---------------------------------------------------------------------*
Initialization.
TEXT01 = 'ABAP List Viewer'.
TEXT02 = 'Radio Buttons'.
Text03 = 'Dynamic Screen Selection'.
*---------------------------------------------------------------------*
* START-OF-SELECTION. *
*---------------------------------------------------------------------*
START-Of-SELECTION.
IF MATRL_R = 'X'.
Select MATNR ERSDA ERNAM LAEDA MTART MATKL MEINS From MARA Into Table IT_MARA Where MATNR In MATNR And ERSDA In Date.
IF IT_MARA IS NOT INITIAL.
Select MATNR MAKTG From MAKT Into Table IT_MAKT For All ENTRIES In IT_MARA Where MATNR = IT_MARA-MATNR And SPRAS = 'E'.
Select MATNR LGORT WERKS From MARD Into Table IT_MARD For All ENTRIES In IT_MARA Where MATNR = IT_MARA-MATNR.
Loop At IT_MARA Into WA_MARA.
WA_FINAL-MATNR = WA_MARA-MATNR.
WA_FINAL-ERSDA = WA_MARA-ERSDA.
WA_FINAL-ERNAM = WA_MARA-ERNAM.
WA_FINAL-LAEDA = WA_MARA-LAEDA.
WA_FINAL-MTART = WA_MARA-MTART.
WA_FINAL-MATKL = WA_MARA-MATKL.
WA_FINAL-MEINS = WA_MARA-MEINS.
Read Table IT_MAKT Into WA_MAKT With Key MATNR = WA_MARA-MATNR.
WA_FINAL-MAKTG = WA_MAKT-MAKTG.
Append WA_FINAL To IT_FINAL.
ENDLOOP.
WA_FCAT-FIELDNAME = 'MATNR'.
WA_FCAT-SELTEXT_M = 'MATERIAL NUMBER'.
WA_FCAT-COL_POS = '1'.
WA_FCAT-Key = 'X'.
WA_FCAT-OUTPUTLEN = '20'.
WA_FCAT-TABNAME = 'IT_FINAL'.
WA_FCAT-HOTSPOT = 'X'.
Append WA_FCAT To IT_FCAT.
Clear WA_FCAT.
WA_FCAT-FIELDNAME = 'MAKTG'.
WA_FCAT-SELTEXT_M = 'MATERIAL DESCRIPTION'.
WA_FCAT-COL_POS = '2'.
WA_FCAT-OUTPUTLEN = '20'.
WA_FCAT-TABNAME = 'IT_FINAL' .
Append WA_FCAT To IT_FCAT.
Clear WA_FCAT.
*---------------------------------------------------------------------*
* SUB ROUTINES *
*---------------------------------------------------------------------*
CALL FUNCTION 'REUSE_ALV_GRID_DISPLAY'
 EXPORTING
 I_CALLBACK_PROGRAM = SY-REPID
 I_CALLBACK_USER_COMMAND = 'INTERACTIVE'
 IT_FIELDCAT = IT_FCAT
 TABLES
 T_OUTTAB = IT_FINAL.
 Else.
 Message S000(Y_MSG) DISPLAY Like 'E'.
 Endif.
Endif.
If MATRL_R = ' '.
Select VBELN ERDAT From VBAK Into Table IT_VBAK Where VBELN In VBELN And ERDAT In Date.
If IT_VBAK Is Not Initial.
Select VBELN POSNR MATNR MATWA From VBAP Into Table IT_VBAP For All ENTRIES In IT_VBAK Where VBELN = IT_VBAK-VBELN.
 WA_FCAT-FIELDNAME = 'VBELN'.
 WA_FCAT-SELTEXT_M = 'SALES DOCUMENT'.
 WA_FCAT-COL_POS = '1'.
 WA_FCAT-Key = 'X'.
 WA_FCAT-OUTPUTLEN = '20'.
 WA_FCAT-TABNAME = 'IT_VBAK'.
 WA_FCAT-Hotspot = 'X'.
 Append WA_FCAT To IT_FCAT.
 Clear WA_FCAT.
 WA_FCAT-FIELDNAME = 'ERDAT'.
 WA_FCAT-SELTEXT_M = 'CREATED ON'.
 WA_FCAT-COL_POS = '2'.
 WA_FCAT-OUTPUTLEN = '20'.
 WA_FCAT-TABNAME = 'IT_VBAK' .
 Append WA_FCAT To IT_FCAT.
 Clear WA_FCAT.
*---------------------------------------------------------------------*
* SUB ROUTINES *
*---------------------------------------------------------------------*
 Call Function 'REUSE_ALV_GRID_DISPLAY'
 Exporting
 I_CALLBACK_PROGRAM = SY-REPID
 I_CALLBACK_USER_COMMAND = 'INTERACTIVE2'
 IT_FIELDCAT = IT_FCAT
 Tables
 T_OUTTAB = IT_VBAK.
 Else.
 Message S000(Y_MSG) DISPLAY Like 'E'.
 Endif.
Endif.
FORM INTERACTIVE USING R_UCOMM TYPE SY-UCOMM R_SELFIELD TYPE SLIS_SELFIELD.
 CLEAR WA_FINAL.
 REFRESH IT_FINAL.
 CLEAR WA_FCAT.
 REFRESH IT_FCAT.
*BREAK-POINT.
 Loop At IT_MARA Into WA_MARA WHERE MATNR = R_SELFIELD-VALUE .
 WA_FINAL-MATNR = WA_MARA-MATNR.
 WA_FINAL-ERSDA = WA_MARA-ERSDA.
 WA_FINAL-ERNAM = WA_MARA-ERNAM.
 WA_FINAL-LAEDA = WA_MARA-LAEDA.
 WA_FINAL-MTART = WA_MARA-MTART.
 WA_FINAL-MATKL = WA_MARA-MATKL.
 WA_FINAL-MEINS = WA_MARA-MEINS.
 Read Table IT_MARD Into WA_MARD With Key MATNR = R_SELFIELD-VALUE.
 WA_FINAL-LGORT = WA_MARD-LGORT.
 WA_FINAL-WERKS = WA_MARD-WERKS.
 Append WA_FINAL To IT_FINAL.
 Endloop.
 WA_FCAT-FIELDNAME = 'MATNR'.
 WA_FCAT-SELTEXT_M = 'MATERIAL NUMBER'.
 WA_FCAT-COL_POS = '1'.
 WA_FCAT-Key = 'X'.
 WA_FCAT-OUTPUTLEN = '20'.
 WA_FCAT-TABNAME = 'IT_FINAL'.
 Append WA_FCAT To IT_FCAT.
 Clear WA_FCAT.
 WA_FCAT-FIELDNAME = 'ERSDA'.
 WA_FCAT-SELTEXT_M = 'CREATED ON'.
 WA_FCAT-COL_POS = '2'.
 WA_FCAT-OUTPUTLEN = '20'.
 WA_FCAT-TABNAME = 'IT_FINAL' .
 Append WA_FCAT To IT_FCAT.
 Clear WA_FCAT.
 WA_FCAT-FIELDNAME = 'ERNAM'.
 WA_FCAT-SELTEXT_M = 'CREATED BY'.
 WA_FCAT-COL_POS = '3'.
 WA_FCAT-OUTPUTLEN = '20'.
 WA_FCAT-TABNAME = 'IT_FINAL'.
 Append WA_FCAT To IT_FCAT.
 Clear WA_FCAT.
 WA_FCAT-FIELDNAME = 'LAEDA'.
 WA_FCAT-SELTEXT_M = 'LAST CHANGE'.
 WA_FCAT-COL_POS = '4'.
 WA_FCAT-OUTPUTLEN = '20'.
 WA_FCAT-TABNAME = 'IT_FINAL' .
 Append WA_FCAT To IT_FCAT.
 Clear WA_FCAT.
 WA_FCAT-FIELDNAME = 'MTART'.
 WA_FCAT-SELTEXT_M = 'MATERIAL TYPE'.
 WA_FCAT-COL_POS = '5'.
 WA_FCAT-OUTPUTLEN = '20'.
 WA_FCAT-TABNAME = 'IT_FINAL'.
 Append WA_FCAT To IT_FCAT.
 Clear WA_FCAT.
 WA_FCAT-FIELDNAME = 'MATKL'.
 WA_FCAT-SELTEXT_M = 'MATERIAL GROUP'.
 WA_FCAT-COL_POS = '6'.
 WA_FCAT-OUTPUTLEN = '20'.
 WA_FCAT-TABNAME = 'IT_FINAL' .
 Append WA_FCAT To IT_FCAT.
 Clear WA_FCAT.
 WA_FCAT-FIELDNAME = 'MEINS'.
 WA_FCAT-SELTEXT_M = 'UNIT OF MEASURE'.
 WA_FCAT-COL_POS = '7'.
 WA_FCAT-OUTPUTLEN = '20'.
 WA_FCAT-TABNAME = 'IT_FINAL'.
 Append WA_FCAT To IT_FCAT.
 Clear WA_FCAT.
 WA_FCAT-FIELDNAME = 'LGORT'.
 WA_FCAT-SELTEXT_M = 'STORAGE LOCATION'.
 WA_FCAT-COL_POS = '8'.
 WA_FCAT-OUTPUTLEN = '20'.
 WA_FCAT-TABNAME = 'IT_FINAL' .
 Append WA_FCAT To IT_FCAT.
 Clear WA_FCAT.
 WA_FCAT-FIELDNAME = 'WERKS'.
 WA_FCAT-SELTEXT_M = 'PLANT'.
 WA_FCAT-COL_POS = '9'.
 WA_FCAT-OUTPUTLEN = '20'.
 WA_FCAT-TABNAME = 'IT_FINAL' .
 Append WA_FCAT To IT_FCAT.
 Clear WA_FCAT.
 CALL FUNCTION 'REUSE_ALV_GRID_DISPLAY'
 EXPORTING
 IT_FIELDCAT = IT_FCAT
 TABLES
 T_OUTTAB = IT_FINAL.
ENDFORM.
FORM INTERACTIVE2 USING R_UCOMM TYPE SY-UCOMM R_SELFIELD TYPE SLIS_SELFIELD.
 CLEAR WA_FINAL1.
 REFRESH IT_FINAL1.
 Clear WA_FCAT.
 Refresh IT_FCAT.
 Loop At IT_VBAP Into WA_VBAP Where VBELN = R_SELFIELD-VALUE.
 WA_FINAL1-VBELN = WA_VBAP-VBELN.
 WA_FINAL1-POSNR = WA_VBAP-POSNR.
 WA_FINAL1-MATNR = WA_VBAP-MATNR.
 WA_FINAL1-MATWA = WA_VBAP-MATWA.
 Append WA_FINAL1 To IT_FINAL1.
 ENDLOOP.
 WA_FCAT-FIELDNAME = 'VBELN'.
 WA_FCAT-SELTEXT_M = 'SALES DOCUMENT'.
 WA_FCAT-COL_POS = '1'.
 WA_FCAT-Key = 'X'.
 WA_FCAT-OUTPUTLEN = '20'.
 WA_FCAT-TABNAME = 'IT_FINAL1'.
 Append WA_FCAT To IT_FCAT.
 Clear WA_FCAT.
 WA_FCAT-FIELDNAME = 'POSNR'.
 WA_FCAT-SELTEXT_M = 'SALES DOCUMENT ITEM'.
 WA_FCAT-COL_POS = '2'.
 WA_FCAT-OUTPUTLEN = '20'.
 WA_FCAT-TABNAME = 'IT_FINAL1' .
 Append WA_FCAT To IT_FCAT.
 Clear WA_FCAT.
 WA_FCAT-FIELDNAME = 'MATNR'.
 WA_FCAT-SELTEXT_M = 'MATERIAL NUMBER'.
 WA_FCAT-COL_POS = '3'.
 WA_FCAT-OUTPUTLEN = '20'.
 WA_FCAT-TABNAME = 'IT_FINAL1'.
 Append WA_FCAT To IT_FCAT.
 Clear WA_FCAT.
 WA_FCAT-FIELDNAME = 'MATWA'.
 WA_FCAT-SELTEXT_M = 'MATERIAL ENTERED'.
 WA_FCAT-COL_POS = '4'.
 WA_FCAT-OUTPUTLEN = '20'.
 WA_FCAT-TABNAME = 'IT_FINAL1' .
 Append WA_FCAT To IT_FCAT.
 Clear WA_FCAT.
 Call Function 'REUSE_ALV_GRID_DISPLAY'
 Exporting
 IT_FIELDCAT = IT_FCAT
 Tables
 T_OUTTAB = IT_FINAL1.
ENDFORM.
1 ACCEPTED SOLUTION

matt
Active Contributor

1. https://blogs.sap.com/2019/05/03/clean-abap/

2.

  • TABLES is obsolete.
  • You're using standard tables rather than hashed (or sorted).
  • You're using FOR ALL ENTRIES instead of JOINs.
  • You're using REUSE_ALV_GRID_DISPLAY instead of CL_SALV_TABLE.
  • You're variables do not have meaningful names
  • The code is badly modularised.
  • The code does not have separation of responsibilities - no layering, no abstraction.
  • The code has no automated tests and isn't in a form where it can easily be brought into a test harness.
  • You're code is using FORMs which are obsolete.
  • You're using prefixes to denote type, which is contrary to SAP's own guidelines and the latest guidelines from DSUG.
  • You've barely any comments, and those that are there are useless. But if you modularise the program properly, then you will need fewer comments anyway.
  • Finally, you've not pretty printed the code, so it's basically unreadable.

Will that be enough to be going along with?

10 REPLIES 10

matt
Active Contributor

1. https://blogs.sap.com/2019/05/03/clean-abap/

2.

  • TABLES is obsolete.
  • You're using standard tables rather than hashed (or sorted).
  • You're using FOR ALL ENTRIES instead of JOINs.
  • You're using REUSE_ALV_GRID_DISPLAY instead of CL_SALV_TABLE.
  • You're variables do not have meaningful names
  • The code is badly modularised.
  • The code does not have separation of responsibilities - no layering, no abstraction.
  • The code has no automated tests and isn't in a form where it can easily be brought into a test harness.
  • You're code is using FORMs which are obsolete.
  • You're using prefixes to denote type, which is contrary to SAP's own guidelines and the latest guidelines from DSUG.
  • You've barely any comments, and those that are there are useless. But if you modularise the program properly, then you will need fewer comments anyway.
  • Finally, you've not pretty printed the code, so it's basically unreadable.

Will that be enough to be going along with?

0 Kudos

Thank you for answering @matthew.billingham.

Please dumb down the following points with examples please.

  • TABLES is obsolete.
  • You're using FOR ALL ENTRIES instead of JOINs.
  • The code is badly modularised.
  • The code has no automated tests and isn't in a form where it can easily be brought into a test harness.
  • Your code is using FORMs which are obsolete.
  • You're using prefixes to denote type, which is contrary to SAP's own guidelines and the latest guidelines from DSUG.

0 Kudos

@matthew.billingham

Okay thanks 🙂

harkirat_kaur
Participant

Hello @SHASHANK SENGAR,

In addition resolution given by @Matthew Billingham. Kindly make use of field symbols and Classes to display ALV.

Also, create structure of Field catalog and pass into FM instead of every time hardcoding the same in report or even for adding extra column to Field catalog at later stage. Just add/append it to structure.

Thanks & Regards,

Sijin_Chandran
Active Contributor

Adding my 2 cents,

If you are having IDES with ABAP 7.4 or + , then try to incorporate new inline syntax in your code.

Try to get more into pure SQL level coding.

Thanks,

Sijin

0 Kudos

Thank you for answering @sijin.chandran.

I have a few questions please help me out.

1. Without using TABLES, how can I mention the source table eg: MARA? (Please show example)

2. "Select MATNR LGORT WERKS From MARD Into Table IT_MARD For All ENTRIES In IT_MARA Where MATNR = IT_MARA-MATNR." In this statement where can I use JOIN?

3. I work in IDES(still learning) and I can't seem to find CL_SALV_TABLE method.

4. Replacement of "FORMS"?

5. Please show me an example of implementing inline syntax using my code.

sengar_2705

Please find my comments inline,

1. Without using TABLES, how can I mention the source table eg: MARA? (Please show example)

Comment: TABLES statement is not mentioning the source table, your SELECTs will work without writing TABLES also. It just creates a work area with same name. Please search on this more to get more clarity. Practice searching things more by yourselves because going ahead you will get lots of queries like this.

2. "Select MATNR LGORT WERKS From MARD Into Table IT_MARD For All ENTRIES In IT_MARA Where MATNR = IT_MARA-MATNR." In this statement where can I use JOIN?

Comment: You can use JOIN for joining MARD and MARA in one Syntax only. Please search on that, its a very common thing.

3. I work in IDES(still learning) and I can't seem to find CL_SALV_TABLE method.

Comment: Its a Class, check in SE24 you should find it.

You can use CL_GUI_ALV_GRID also,

Please search in SE38 as BCALV_GRID_* , you will get a list of Standard SAP sample codes.

4. Replacement of "FORMS"?

Comment: Not replacement, in S4 HANA more preference are given to Adobe Forms with XML based Interface ( Odata ). So Please try to explore it more.

5. Please show me an example of implementing inline syntax using my code.

Comment: You can get lot of contents over here.

Please search and explore.

Thanks,

Sijin

matt
Active Contributor

sijin.chandran

The replacement of forms is not the replacement of sapscript/smartforms, it is replacement of PERFORM/FORM subroutines. The replacement is function modules and classes with methods - and moving to object oriented programming.

matthew.billingham

Ohh I got it wrong with "FORMS", I thought he was referring to Printing/Spool provisions of SAP.

Thanks for the correction.

0 Kudos

Thanks for answering @sijin.chandran.

In my code, if you comment out TABLES, the SELECT statement will show an error. So, what changes should be done?

Sir, it's very difficult to search for exact queries, everyone has their own way of writing codes. If you could tell me what exactly I have to replace to get the code right is better than writing "Please search on that, it is a very common thing". With all due respect, Sir, I have searched and tried all the possibility to my knowledge before asking this question.