Let’s start with three questions about the existing ABAP code:
- Features: Can I add them easily without breaking anything?
- Bugs: can I fix them without breaking anything?
- Users ask a query on custom development: can I explain the functionality after reading the code?
Businesses/SI/AMS partners who are in multiple ongoing SAP projects focus on the delivery timeline only and mostly don’t introduce a must-required role/stage in the project as Code approver/peer reviewer.
Post-Go-Live, those implementations/enhancements start facing difficulties like inconsistent code, AMS partners face challenges in code readability for feature scalability or bug fixing, poor performance in custom reports/utilities/assets/automation or standard transactions with multiple enhancements due to high response time (wait time + execution time), code maintenance, etc.
Benefits of Code Reviewer role & Peer review task in the project:
- Clean code with comments to avoid complexity and increase the readability of the code
- The code is well written by considering the guidelines declared by the project like Headers (at start & end of the code) including Transport & RICEFW number, use of Function modules instead of code repetition, etc. with a common understanding of code style which helps to ramp up new developers more quickly and focus the time on pair programming or code reviews on other aspects.
- Considered performance of the code like SELECT statement is outside the LOOP, used CASE instead of multiple IF-ELSE, used new ABAP syntax, etc.
- any many more.
Peer code review is also useful to increase the developer’s knowledge by sharing codes for review and providing comments as different people see the code from different perspectives.
The code reviewer has many responsibilities:
- check and review whether Peer review is completed or not with set benchmarks
- There is no hardcoding in the code by using TVARVC, SETLEAF, or rule table
- Code formatting
- use of message classes & standard texts (SO10)
- Performance parameters
- Golden rules are followed or not
- Clean coding which a solution architect gained from his experience (some are mentioned below)
- Don’t declare inline in optional branches
IF has_entries = abap_true.
DATA(value) = 1.
ELSE.
value = 2.
ENDIF.
This works fine because ABAP handles inline declarations as if they were at the beginning of the method. However, it is extremely confusing for readers, especially if the method is longer and you don’t spot the declaration right away. In this case, break with inlining and put the declaration up-front:
DATA value TYPE i.
IF has_entries = abap_true.
value = 1.
ELSE.
value = 2.
ENDIF.
- Chaining Variables declaration
DATA:
name TYPE seoclsname,
reader TYPE REF TO reader.
Instead of,
DATA name TYPE seoclsname.
DATA reader TYPE REF TO reader.
- No Empty IF branches like:
IF has_entries = abap_true.
ELSE.
- Prefer CASE to ELSE IF for multiple alternative conditions
Download Golden Rules and the cheat sheet for code guidelines.
