Scrutinizing ABAP GenAI example from SAP Sapphire keynote

Below is the code example from yesterdays SAP Sapphire Keynote,

At the point of writing, its not possible to replay the keynote, as it was taken down from YouTube, and not available on the Sapphire Virtual homepage.

But, given a prompt the above code was generated. This happened at the main keynote, the feature was announced 6+ months ago, and this being a keynote I think its okay to look into every detail of it. At a keynote SAP has full control of what to show and how, and I do assume there is a review step as to what goes into the keynote.

On top of this, SAP advocates clean core, clean code, clean everything, so what SAP shows on the keynote I assume SAP certifies as clean.

Lets go,

A: Indentation is off

Indentation in ABAP is typically 2 spaces, looking at line 2 its one space, while line 14 and 15 are actually aligned vertically its still one off. Line 19 and 20 doesn't follow the lines after it inside the IF

B: Hungarian notation mix

Using Hungarian notation or not is a big topic in ABAP. However most agree that mixing both in the same code is bad practice.

C: Parentheses

In line 20, the leftmost and right most parentheses can be removed.

D: Types

Using "TYPE p DECIMALS 2" is not something I've seen very often, and looking in the parameters indicates that the fields are "Decfloat" which is a different type.

This might be an error and worst case cause dumps, might also be okay. I don't have access to a system to verify.

E: lv_error is unused

The variable "lv_error" is unused

F: Comments

Like line 1, adding a comment that the DATA is declared below it, no need to. Add comments where it adds value, don't replicate whats below it

H: CORRESPONDING #

MOVE-CORRESPONDING works, but CORRESPONDING # is more modern, and considered cleaner by some developers

I: Inline declarations

In modern ABAP, variables can be declared inline, increasing readability and also helping make sure the types are correct

J: Line 11?

Line 11 jumps out, as it moves "element attributes" to "result amounts", this might be okay, I guess its used to fill some key fields in the result amounts. But it might risk moving too much depending on the types(which I dont have access to check)

K: Use "-="

The modern subtracting move "-=" can be used to reduce the amount of code

Just put the example into https://playground.abaplint.org, and it will hint the developer with many of the above issues.

So, what about something like the below, did I forget anything?

item_result_amounts = CORRESPONDING #( item_amounts ).
item_result_attributes = CORRESPONDING #( item_attributes ).
item_result_quantities = CORRESPONDING #( item_quantities ).
prcg_element_result_amounts = CORRESPONDING #( prcg_element_attributes ).
active_price_result_amounts = CORRESPONDING #( active_price_amounts ).

DATA(lv_temperature) = item_attributes-yy_temperature_pci.

" when the weather is good, people buy more ice cream, 
" so we reduce the price
IF lv_temperature > 20.
  prcg_element_result_amounts-conditionamount -= 
    ( lv_temperature - 20 ) / 2 ) * 
    ( prcg_element_attributes-conditionamount / 10 ).
ENDIF.