Better coding standards.

[Former Member]

SELECT   docnum credat
           FROM edidc
           INTO
           TABLE it_edidc
           WHERE credat IN s_credat     "Created on
           AND   status = '51'          "Application document not posted
           AND   rcvpor = 'SAPDV1'      "Receiver port
           AND   direct = '2'.          "Inbound
  IF sy-subrc NE 0.
    MESSAGE i000 DISPLAY LIKE 'E' WITH text-003.
    EXIT.
  ENDIF.

  SORT it_edidc BY docnum ASCENDING.

  IF NOT it_edidc[] IS INITIAL.
    SELECT docnum sdata
           FROM edid4
           INTO TABLE it_edid4
           FOR ALL ENTRIES IN it_edidc
           WHERE docnum = it_edidc-docnum
           AND   segnum = '1'.
  ENDIF.

  LOOP AT it_edidc INTO wa_edidc.
    MOVE-CORRESPONDING wa_edidc TO wa_output.
    READ TABLE it_edid4 INTO wa_edid4
                        WITH KEY docnum = wa_edidc-docnum
                        BINARY SEARCH.
    IF sy-subrc EQ 0.
      MOVE:   wa_edid4-sdata  TO  wa_output-sdata.
    ENDIF.

    APPEND wa_output TO it_output.
    CLEAR : wa_output.
  ENDLOOP.

Is this proper coding or we can optimize it more. Please give some solutions for this code any better coding standards?

[venkat_o]

<li>You are checking whether FOR ALL ENTRIES table is empty or not .. Right <li>FOR ALL ENTRIES table is being sorted also based on key. <li>Reading table it_edid4 with binary search. What else need.. but small request.. you missed small thing I don't think that affects the result but still that is better if you sort it_edid4 table when you use BINARY SEARCH on the table. Sort before you loop it_edidc table. Thanks Venkat.O

[Former Member]

Dear John,

Few suggestions:

1. As can be seen from the code, you are interested in only two fields from the table EDIDC namely docnum and credat.

What I would suggest here is that you can create a local type having only these two fields and then declare the internal table and a corresponding field symbol.

SELECT   docnum credat
           FROM edidc
           INTO
           TABLE it_edidc
           WHERE credat IN s_credat     "Created on
           AND   status = '51'          "Application document not posted
           AND   rcvpor = 'SAPDV1'      "Receiver port
           AND   direct = '2'.          "Inbound

2. You do not need explicitly put ASCENDING. This is by default.

  SORT it_edidc BY docnum ASCENDING.

3. same as step : You can create a local type, a corresponding internal table and a field-symbol for the table 'it_edid4'.

4.

  LOOP AT it_edidc INTO wa_edidc.
    MOVE-CORRESPONDING wa_edidc TO wa_output.
    READ TABLE it_edid4 INTO wa_edid4
                        WITH KEY docnum = wa_edidc-docnum
                        BINARY SEARCH.
    IF sy-subrc EQ 0.
      MOVE:   wa_edid4-sdata  TO  wa_output-sdata.
    ENDIF.
 
    APPEND wa_output TO it_output.
    CLEAR : wa_output.
  ENDLOOP.

Instead of this, you can try the following:

As suggested by Venkat, sort the table 'it_edid4' by docnum. This will help when you perform the BINARY SEARCH on this.

  LOOP AT it_edidc ASSIGNING <fs_edidc>.
"MOVE-CORRESPONDING wa_edidc TO wa_output.
" Refrain from using MOVE-CORRESPONDING
" Try populating field by field.
    
"READ TABLE it_edid4 INTO wa_edid4
"WITH KEY docnum = wa_edidc-docnum
" use field symbol instead
READ TABLE it_edid4 ASSIGNING <fs_edid4>
                        WITH KEY docnum = <fs_edidc>-docnum
                        BINARY SEARCH.
    IF sy-subrc EQ 0.
"MOVE:   wa_edid4-sdata  TO  wa_output-sdata.
    wa_output-sdata = <fs_edid4>-sdata 
    ENDIF.
 
    APPEND wa_output TO it_output.
    CLEAR : wa_output.
  ENDLOOP.

Hope you find this useful..:)

Regards

s@k

Edited by: siemens.a.k on Feb 5, 2010 11:41 AM

[Former Member]

Hi John,

" Honestly there is no straight answer for this Question

SORT it_edidc BY docnum ASCENDING.
DELETE ADJACENT DUPLICATES FROM it_edidc COMPARING docunum " This will reduce the load on DB 
" For More info SE38 --> Environment -> Examples --> Performance
" Try to avoid MOVE-CORRESPONDING wherever possible and go through the Above Examples
  IF NOT it_edidc[] IS INITIAL.
    SELECT docnum sdata
           FROM edid4
           INTO TABLE it_edid4
           FOR ALL ENTRIES IN it_edidc
           WHERE docnum = it_edidc-docnum
           AND   segnum = '1'.
  ENDIF.

Cheerz

Ram

[Former Member]

hi,

1. In select statement you can select key fields ......by selecting them Primary index will trigger,
 it will improve the performance.
2. Do not use hard coded values in place of that you can use constants.
3. before fill the table through select statement REFRESH the internal table .

Regards

Gaurav

[Former Member]

1)Use condtion " IF NOT it_edidc[] IS INITIAL" commonly for :-

SORT it_edidc BY docnum ASCENDING and

LOOP AT it_edidc INTO wa_edidc.

2) remove hardcoding

[Former Member]

This message was moderated.