Need clarification from experts regarding few code review comments received

[Former Member]

Hi,

I know that this is a very trivial question to post onto in here but just wanted to double check with all the experts out there. I was lately given few code review comments by one of the senior developers in my project which I couldnt completely agree upon. I thought to seek further inputs from the experts out here.

1) Consider the SQL query below:

SELECT  orgeh orgtx FROM t527x INTO TABLE ch_org_unit_descs
               FOR ALL ENTRIES IN ch_org_unit_list WHERE sprsl = gc_english AND
                                                         orgeh = ch_org_unit_list-orgeh AND
                                                         endda = gc_endda.

I was instructed not to pass any constant values (like how am passing to SPRSL & ENDDA) as shown above. The code reviewer said that this would result in poor performance & that am supposed to perform a DELETE upon the resulting internal table instead. I did try read around & search the available documentation but could not come up with any similar note anywhere. Can the experts please comment on the credibility of this statement?

2) Also say I am writing a CASE statement then if I say as WHEN 'ATT' does this mean that am hard coding 'ATT' and that I should have gone for a constant instead? Whilst the code reviewer said that I was hard coding text I would have to admit that I haven't ever declared a constant for handling any of the WHEN statements in any of my projects till date. Is this a bad practice from my end or do you think its like imposing double standards?

Regards,

Uday

[Former Member]

1) You are correct.

2) It would probably be better to declare a constant. That way if you have 'ATT' coded in multiple places in the program and you have to change it, you only change it in one place..

Rob

[uday_gubbala2]

Hi Rob,

Thanks for following up with my query. So checking for a constant within the FOR ALL ENTRIES leads to poor performance? This is something very new to me. Can you please help redirect me to any further documentation available on this? I did try search around on SDN & help.sap.com but all I could find regarding this were:

1) Check that the internal table is not empty

2) Do a sort & delete adjacent duplicates before using the statement

I guess I should be thankful to the code reviewer & yourself for putting me onto the proper path.

Regards,

Uday

[Former Member]

If checking for a constant within FOA is a performance problem, it's news to me.

Much of what you are asking can be tested to get the answer.

Rob

[Former Member]

1) remove the constant conditions from your select statement. .keep the FOA condition only.. and after your select statement you can:

delete ch_org_unit_descs
WHERE sprsl NE gc_english OR
    endda NE gc_endda.

[Sandra_Rossi]

1) From my point of view, having constants in the FOR ALL ENTRIES is not really a performance issue.

As SAP converts the open sql SELECT into several native sql SELECT, depending on internal table content and profile parameters, we get:

SELECT orgeh orgtx FROM t527x WHERE sprsl = 'E' AND orgeh IN ('value1','value2','value3'...) AND endda = 'date'.

SELECT orgeh orgtx FROM t527x WHERE sprsl = 'E' AND orgeh IN ('value11','value12','value13'...) AND endda = 'date'.

etc.

Even if T527X is buffered, the FOR ALL ENTRIES should be able to access it in the buffer.

So, I don't see where the peformance issue is.

2) Though defining all literals as constants is a well-known standard, I don't think it brings clarity and well-written programs. I often see CONSTANTS a TYPE c VALUE 'A', CONSTANTS one TYPE i VALUE 1. We should use a constant when a value is often used or when a name is better than just a meaningless value.

[SuhaSaha]

I second both Sandra & Rob on their views on FAE. As Rob has suggested do a runtime analysis or SQL trace and compare the DB execution times.

>

> We should use a constant when a value is often used or when a name is better than just a meaningless value.

Using constants for any literal used in the code makes it difficult to understand. I have seen codes with something like CALL TRANSACTION c_fb01 or AUTHORITY OBJECT c_s_dataset.

I always make it a point to declare constants for organizational or functional values which might change in future. Declaring constants for elements which are specific to my code is definitely a NO-NO for me. Comments are welcome on this.

BR,

Suhas