Loop taking too long

[Former Member]

Hi Experts

I have written the following code so that it extracts from a table (zactivities) into an internal table it_rrr so that the titles 'Total Completed', 'Comp Within Target', and 'Percentage Completed' appear for each ACT_GRP in the internal table. It seems it's taking too long to process the loop. Can you suggest how I can solve this?

select act_grp from zactivities into it_rrr-act_grp.
  append it_rrr.
endselect.
 
delete adjacent duplicates from it_rrr comparing act_grp.
 
loop at it_rrr.
  at new act_grp.
    st_rrr = it_rrr.
    act_grp = st_rrr-act_grp.
    it_rrr-comp_text = 'Total Completed'.
    modify it_rrr.
    clear st_rrr.
    st_rrr-comp_text = 'Comp Within Target'.
    st_rrr-act_grp = act_grp.
    append st_rrr to it_rrr.
    clear st_rrr.
    st_rrr-act_grp = act_grp.
    st_rrr-comp_text = 'Percentage Completed'.
    append st_rrr to it_rrr.
  endat.
  clear act_grp.
  clear st_rrr.
endloop.
 
sort it_rrr by act_grp.

Regards,

Darlington

[Former Member]

select act_grp from zactivities into it_rrr-act_grp.
  append it_rrr.
endselect.

sort it_rrr by act_grp.
delete adjacent duplicates from it_rrr comparing act_grp.
 
loop at it_rrr assigning <fs>.
  at new act_grp.
    idx = sy-tabix.   
    act_grp = <fs>-act_grp.
    <fs>-comp_text = 'Total Completed'.

    wa_rrr = <fs>.
    st_rrr-act_grp = act_grp    
    st_rrr-comp_text = 'Comp Within Target'.
    idx = idx + 1.
    insert st_rrr INTO it_rrr index idx.

    st_rrr-act_grp = act_grp.
    st_rrr-comp_text = 'Percentage Completed'.
    idx = idx + 1.
    insert st_rrr INTO it_rrr index idx.

  endat.
endloop.

Your coding is not really correct, you add additional lines, but they contain only different texts, no different values.

This is not a good idea.

The sort must come first, otherwise it can not work, and later you must no destroy the sort order !!!!!

Your appends create new ' at new act_grp.'

actually you code should never stop.

Siegfried

[Former Member]

Hi,

• Assumption:

• it_rrr has two fields: act_grp & comp_text

SELECT act_grp from ZACTIVITIES INTO TABLE it_rrr.

• Here it_rrr should have only act_grp as the field

sort it_rrr by act_grp.

delete adjacent duplicates from it_rrr comparing act_grp.

FIELD-SYMBOLS : <fs_w_rrr> LIKE LINE OF it_rrr.

• Declare the Field symbol on the TYPE of it_rrr

loop at it_rrr assigning <fs_w_rrr>.

• at new act_grp.

• At new is not required, as you have done the delete adjacent duplicates

• based on act_grp

• for modifying contents, use as below

<fs_w_rrr>-comp_text = 'Total Completed'

endloop.

But i didnt understand why you need to append the same table and loop on the same. It will be a never ending loop right?

Regards,

Chathia.

[Former Member]

* st_rrr and it_rrr declared and defined elsewhere

clear st_rrr.

select distinct act_grp into (st_rrr-act_grp) from zactivities.
  st_rrr-comp_text = 'Total Completed'.
  append st_rrr to it_rrr.
  st_rrr-comp_text = 'Comp Within Target'.
  append st_rrr to it_rrr.
  st_rrr-comp_text = 'Percentage Completed'.
  append st_rrr to it_rrr.
  clear st_rrr.
endselect.

In general:

- Never use SELECT ... ENDSELECT loops to simply fetch and store data. INTO TABLE is more efficient.

- If you will do a single fetch and processing pass, it might be more efficient to use the SELECT ... ENDSELECT with processing logic in the middle, instead of INTO TABLE followed by LOOP.

- Fetching data from the DB, then sorting and removing duplicates is inefficient. SELECT DISTINCT lets the DB do that work for you and reduces the volume of data that has to be transfered and processed.

- Don't loop over and append to the same table.

- Avoid using table headers followed by MODIFY. Clear use of structures or field symbols instead of table headers makes for clearer code, and less chance for errors. Mixing the two is certainly asking for trouble.

[Former Member]

Hi Derick,

I will definitely use your suggestions in the future. Thank you very much.

Regards

[Former Member]

Hi Derick,

I will definitely remember that. Thanks for the very valuable information.

God Bless.

[Former Member]

> Never use SELECT ... ENDSELECT loops to simply fetch and store data. INTO TABLE is more efficient.

this is repeated again and again, but it is not true ...

This was discussed so often, and can be easily shown!!!!!!!!!

The point here is the logic with the never ending loop.

And actually the logic itself is strange, there are three identical lines with three different texts. What it is the point of that, it does not make much sense.

That is what you should remember.

[Former Member]

this is repeated again and again, but it is not true ... 
This was discussed so often, and can be easily shown!!!!!!!!!

I have since read the sticky thread on that topic, and drew my own conclusions. In the interests of community standard, I appreciate the need to not rehash ground well covered.

Clearly you are an active and respected member of this community, so no direspect, but why react to only one element of the advice I offered and by not commenting on the rest throw it all into doubt?

[Former Member]

Hi,

The Main Problem with u r code is :-

if u use LOOP on Internal table with Append or Insert To same table makes looping on same table multiple times from first line once new line got added to internal table.

thats the problem.

So u can do modify on same table will not makes execution difference,

but Inser and Append on same table with in same table Loop will makes lot of difference with EXECUTION, i.e. from first line of internal table re-execution starts and its re-executing first line after operation.

u can do best way is use modify on IT_RRR and append and insert to other internal tables it_rrr1 and IT_RRR2

then once operation completed append IT_RRR1 and IT_RRR2 to IT_RRR.

loop at it_rrr.

at new act_grp.

st_rrr = it_rrr.

act_grp = st_rrr-act_grp.

it_rrr-comp_text = 'Total Completed'.

modify it_rrr.

clear st_rrr.

st_rrr-comp_text = 'Comp Within Target'.

st_rrr-act_grp = act_grp.

append st_rrr to it_rrr1. <----

clear st_rrr.

st_rrr-act_grp = act_grp.

st_rrr-comp_text = 'Percentage Completed'.

append st_rrr to it_rrr2.

endat.

clear act_grp.

clear st_rrr.

endloop.

append lines of it_rrr1 to it_rrr.

append lines of it_rrr2 to it_rrr.

refresh : it_rrr1, it_rrr2.

Best Regards,

Vijay Mekala