ABAP Code Review Checklist That Finds Real Defects

Decision context

If your review checklist repeats what ATC checks automatically, the review is a slower version of a tool. Run the tool first, then spend the human attention on what the tool cannot see: whether the code does the right thing, whether it fails safely, and whether the next person can change it.

The items below are ordered by how often they cause a production incident in SAP specifically. Selection performance and authorization checks lead, because both are invisible in a development system with small data and wide authorizations.

Adranum sits next to this rather than inside it. It tells the reviewer what the change reaches, which tests intersect it, and what is uncovered, so the review is scoped by impact rather than by diff size.

Correctness and failure behavior

The reviewer's first job is to establish what the code does when things are not normal, because the happy path was already tested by the developer.

• What happens when the select returns nothing. An empty result that falls through to a default is a common silent defect.
• Are the return codes and exceptions actually handled, or caught and swallowed? A catch block with no handling is a defect hiding in plain sight.
• Does an error leave the data consistent? Partial updates without a rollback are the expensive class.
• Are units, currencies, and quantities carried with their values rather than assumed? Currency conversion and rounding are where amounts quietly go wrong.
• Are date and time zone assumptions explicit, particularly where the code compares dates recorded in different systems?
• Is the code idempotent where it can be called twice? Interfaces and background jobs get called twice.

Performance, in the places it matters in SAP

A development system with a thousand records will not reveal any of these. Review is the last chance before production volume does.

• Selects inside loops. This is still the single most common cause of an ABAP performance incident.
• Selection without a usable index, and selection of full rows where a few fields are needed.
• Missing restrictions on client, company code, or organisational unit, which is both a performance and a correctness problem.
• Internal table types and key definitions that do not match the access pattern, particularly linear reads on large standard tables.
• Nested loops over data sets that scale with production volume rather than with test volume.
• For code running on SAP HANA, work pushed into the application layer that the database could do, and the reverse where a database-side operation defeats the intended index.
• Anything running in a dialog work process that will exceed the runtime limit at production volume.

Security and authorization

Authorization defects almost never appear in a development system, because developers hold authorizations the business does not.

• Is there an authorization check, and is it on the right object and the right activity for what the code actually does?
• Is the check done before the action rather than after the data is already displayed?
• Dynamic table and field names, dynamic SQL, and generated code, all of which move a check the compiler could make into runtime.
• Injection risk wherever user input reaches a dynamic clause, an OS command, or a path.
• Hardcoded credentials, keys, endpoints, and file paths, which are always a defect regardless of the environment they were written for.
• Whether logging records enough for an audit without recording data that must not be logged.

Maintainability, and the checks a tool cannot make

These are judgement calls, which is exactly why they belong to the human reviewer.

• Does the code belong here at all? A rule implemented in three places is a design problem the review can still catch cheaply.
• Could this have been configuration rather than code? This question deletes more custom code than any remediation project.
• Is the modification a modification when an enhancement would do? Modifications return at every upgrade.
• Is there a test for the logic, and does the test assert the business rule rather than the implementation?
• Will the next person understand why, not just what? Comments explaining the business reason age far better than comments explaining the syntax.
• Is the change complete as a transport: dictionary dependents, table entries, variants, roles, and text elements included?

What the workflow must cover

• Impact-scoped review. Show what the change reaches, so the reviewer's attention goes to the risky part of the diff rather than to its largest part.
• Coverage in the review. Report which impacted paths have tests and which do not, at the moment the review decision is made.
• Tool-first ordering. Run static analysis before the human review so the review does not repeat what the tool already reported.
• Completeness check. Verify the transport carries the dictionary dependents, entries, variants, roles, and texts the change needs.

Implementation workflow

Start with a bounded customer scenario and explicit acceptance criteria. Preserve native SAP permissions and accountable review while the software creates a repeatable evidence chain.

1. Run the static analysis variant against the transport first.
2. Review correctness and failure behavior before anything else.
3. Check performance at production volume assumptions rather than test volume.
4. Check authorization on the object, activity, and timing.
5. Decide the maintainability questions, then confirm transport completeness.

Evidence to require

A transformation claim should resolve to observable artifacts, decisions, and execution receipts. Ask for the following evidence in a representative evaluation:

• Static analysis result for the transport
• Reviewer notes per category
• Authorization check location and object
• Performance assumptions at production volume
• Test presence and assertions
• Configuration alternative considered
• Transport completeness check
• Review decision and reviewer

Boundaries and non-claims

Adranum separates analysis, proposal, human review, package creation, customer-local validation, and production execution. A later state never rewrites the evidence that supported an earlier decision.

• A review is a sample of attention. It reduces defect probability, it does not eliminate it.
• Performance judgements at review time are assumptions about production volume, and only measurement settles them.
• Whether a requirement should be code at all is a business and architecture decision that a reviewer can raise but not decide alone.

Buyer checklist

• Was static analysis run before the human review?
• What does the code do when the select returns nothing?
• Where is the authorization check, and is it before the action?
• What happens to this code at production volume?
• Could this have been configuration instead?