Code Review Best Practices fuer ABAP

Code Reviews sind ein wesentlicher Bestandteil professioneller Softwareentwicklung. In der ABAP-Welt gewinnen sie durch moderne Tools wie abapGit und ADT-Integration zunehmend an Bedeutung. Dieser Artikel zeigt, wie du Code Reviews effektiv gestaltest und dabei die Codequalitaet sowie die Teamkultur verbesserst.

Der Wert von Code Reviews

Code Reviews bringen weit mehr als nur Fehlersuche:

Statistiken zu Code Reviews

Studien zeigen konsistent den Wert von Code Reviews:

• Code Reviews finden 60-70% der Defekte vor dem Testing
• Ohne Reviews kommen etwa 15 Fehler pro 1000 Codezeilen in Produktion
• Mit Reviews sinkt diese Zahl auf etwa 5 Fehler
• Die fruehe Fehlererkennung spart erhebliche Kosten gegenueber spaeter Korrektur

Checkliste fuer ABAP Code Reviews

Eine strukturierte Checkliste hilft, konsistente Reviews durchzufuehren.

1. Funktionalitaet

• Erfuellt der Code die Anforderungen?
• Sind alle Akzeptanzkriterien abgedeckt?
• Wurden Edge Cases beruecksichtigt?
• Ist die Fehlerbehandlung vollstaendig?

2. Code-Qualitaet

• Sind Namen aussagekraeftig (Variablen, Methoden, Klassen)?
• Sind Methoden kurz und fokussiert (Single Responsibility)?
• Gibt es Code-Duplikation (DRY-Prinzip)?
• Ist der Code gut lesbar ohne viele Kommentare?

3. ABAP-spezifisch

• Werden moderne ABAP-Konstrukte verwendet?
• Sind SELECT-Statements performant (Indizes, Aggregationen)?
• Wird auf internen Tabellen effizient gearbeitet?
• Sind Authority Checks vorhanden wo noetig?

4. Sicherheit

• Keine SQL-Injection Moeglichkeiten?
• Authority Checks fuer sensible Operationen?
• Keine hartcodierten Credentials?
• Input-Validierung vollstaendig?

5. Testbarkeit

• Gibt es ABAP Unit Tests?
• Sind die Tests aussagekraeftig?
• Ist der Code testbar strukturiert (Dependency Injection)?
• Sind Test-Doubles fuer externe Abhaengigkeiten vorhanden?

Clean ABAP Regeln fuer Reviews

Bei Code Reviews sollten die Clean ABAP Guidelines systematisch geprueft werden.

Naming Conventions pruefen

" Review-Kommentar: Name zu generisch
DATA: lv_val TYPE i.

" Besser: Aussagekraeftiger Name
DATA: lv_total_amount TYPE i.

Typischer Review-Kommentar:

“Der Variablenname lv_val ist nicht aussagekraeftig. Was wird hier gespeichert? Bitte einen beschreibenden Namen wie lv_order_count oder lv_total_amount verwenden.”

Methodenlaenge pruefen

" Review-Kommentar: Methode zu lang, mehrere Verantwortlichkeiten
METHOD process_order.
  " 200 Zeilen Code...
  " Validierung
  " Berechnung
  " Datenbankzugriff
  " Ausgabe
ENDMETHOD.

Typischer Review-Kommentar:

“Diese Methode hat 200+ Zeilen und mehrere Verantwortlichkeiten. Bitte in kleinere Methoden aufteilen: validate_order(), calculate_totals(), save_to_database(), prepare_output().”

Boolean Parameter vermeiden

" Review-Kommentar: Boolean-Parameter unklar
CALL METHOD process_data( iv_flag = abap_true ).

Typischer Review-Kommentar:

“Boolean-Parameter sind schwer lesbar. Was bedeutet iv_flag = abap_true? Besser: Separate Methoden wie process_data_in_test_mode() oder einen Enum-Typ verwenden.”

Exceptions statt Return Codes

" Review-Kommentar: Return Code statt Exception
METHOD get_customer.
  IF customer_not_found.
    rv_success = abap_false.
    RETURN.
  ENDIF.
ENDMETHOD.

Typischer Review-Kommentar:

“Bitte Exceptions statt Return Codes verwenden. Exceptions machen den Fehlerfall explizit und erzwingen die Behandlung. Beispiel: RAISE EXCEPTION TYPE cx_customer_not_found.”

Moderne Syntax verwenden

" Review-Kommentar: Veraltete Syntax
DATA: lt_orders TYPE TABLE OF zorder.
CLEAR lt_orders.
REFRESH lt_orders.
MOVE wa_order TO lt_orders.
APPEND lt_orders.

" Empfohlen: Moderne Syntax
DATA(lt_orders) = VALUE zorder_tt( ).
APPEND wa_order TO lt_orders.

Typischer Review-Kommentar:

“Bitte moderne ABAP-Syntax verwenden. MOVE ... TO und REFRESH sind veraltet. Inline-Deklarationen mit DATA() und VALUE-Konstruktor sind lesbarer.”

Konstruktives Feedback geben

Die Art, wie Feedback gegeben wird, ist entscheidend fuer die Teamdynamik.

Do’s beim Feedback

Don’ts beim Feedback

Beispiele fuer gute Review-Kommentare

Statt:

“Das ist falsch, man macht das nicht so.”

Besser:

“Diese Loesung funktioniert, aber ich sehe ein Risiko bei grossen Datenmengen. Hast du ueberlegt, die Verarbeitung in Paketen zu machen? Beispiel: LOOP AT lt_data ASSIGNING FIELD-SYMBOL(<ls_data>) PACKAGE SIZE 1000.”

Statt:

“Schlechte Benennung.”

Besser:

“Der Methodenname do_it() verraet nicht, was die Methode macht. Wie waere es mit calculate_order_total() oder send_notification()?”

Feedback-Kategorien

Nutze Labels, um die Wichtigkeit zu signalisieren:

Code Review Tools fuer ABAP

ADT (ABAP Development Tools)

ADT in Eclipse bietet grundlegende Review-Unterstuetzung:

Vergleichsfunktionen:

• Objektvergleich zwischen Systemen (Development vs. Quality)
• Versionshistorie im Transportwesen
• Lokale Aenderungshistorie

Navigation:

• Call Hierarchy fuer Impact-Analyse
• Where-Used-Lists fuer Aenderungsauswirkungen
• Quick-Fix Vorschlaege fuer Common Issues

ATC Integration:

• ABAP Test Cockpit direkt in der IDE
• Sofortige Rueckmeldung zu Code-Qualitaet
• Custom Checks definierbar

abapGit fuer Code Reviews

abapGit ermoeglicht moderne Pull-Request-basierte Reviews:

Workflow mit abapGit:

1. Feature Branch erstellen

   Repository > Branch > Create Branch

2. Code entwickeln und committen

   Repository > Stage > Commit

3. Push zu Remote Repository

   Repository > Push

4. Pull Request erstellen (auf GitHub/GitLab)

   • Beschreibung der Aenderungen
   • Reviewer zuweisen
   • Labels hinzufuegen

5. Review durchfuehren

   • Diff-Ansicht nutzen
   • Inline-Kommentare schreiben
   • Approve/Request Changes

6. Merge nach Approval

   • Squash Merge fuer saubere Historie
   • Branch loeschen

Vorteile von abapGit-Reviews:

SAP-eigene Tools

ABAP Test Cockpit (ATC):

• Automatische Statische Code-Analyse
• Integrierbar in Review-Prozess
• Custom Checks moeglich

Transport Management:

• Workflow fuer Freigaben
• Vier-Augen-Prinzip
• Dokumentation der Aenderungen

Code Review in CI/CD integrieren

CI/CD Pipelines koennen Review-Prozesse automatisieren und absichern.

Automatische Checks vor Review

# Beispiel: GitHub Actions Workflow
name: ABAP Code Quality

on:
  pull_request:
    branches: [ main, develop ]

jobs:
  quality-check:
    runs-on: ubuntu-latest
    steps:
      - name: Run ATC Checks
        uses: sap/abap-atc-action@v1
        with:
          atc-check-variant: 'STANDARD'
          fail-on-findings: true

      - name: Run ABAP Unit Tests
        uses: sap/abap-unit-action@v1
        with:
          test-package: 'ZTEST_PACKAGE'

      - name: Check Code Coverage
        uses: sap/abap-coverage-action@v1
        with:
          minimum-coverage: 70

Branch Protection Rules

Konfiguriere Branch Protection fuer erzwungene Reviews:

Review-Metriken

Messe die Effektivitaet deines Review-Prozesses:

Typische Review-Kommentare mit Beispielen

Performance

Problem: Nested SELECT in Loop

LOOP AT lt_orders INTO ls_order.
  SELECT SINGLE * FROM zcustomer INTO ls_customer
    WHERE customer_id = ls_order-customer_id.
ENDLOOP.

Review-Kommentar:

“[BLOCKING] SELECT in LOOP verursacht N+1 Query Problem. Bitte alle Customer-IDs sammeln und mit einem SELECT mit FOR ALL ENTRIES abfragen:

SELECT * FROM zcustomer INTO TABLE lt_customers
  FOR ALL ENTRIES IN lt_orders
  WHERE customer_id = lt_orders-customer_id.

Dann mit READ TABLE im Loop zugreifen.”

Fehlerbehandlung

Problem: Leerer CATCH Block

TRY.
    process_critical_data( ).
  CATCH cx_root.
    " Nichts tun
ENDTRY.

Review-Kommentar:

“[BLOCKING] Leerer CATCH-Block verschluckt Fehler. Bitte entweder:

• Exception loggen: cl_demo_output=>write( cx_root->get_text( ) )
• Exception weiterwerfen: RAISE EXCEPTION cx_...
• Bewusst ignorieren mit Kommentar: " Fehler wird ignoriert weil ...”

Sicherheit

Problem: Dynamisches SQL

DATA(lv_where) = |FIELD = '{ iv_user_input }'|.
SELECT * FROM ztable INTO TABLE @lt_result WHERE (lv_where).

Review-Kommentar:

“[BLOCKING] SQL-Injection Gefahr! Benutzereingabe iv_user_input wird direkt im SQL verwendet. Bitte Parameterbindung nutzen:

SELECT * FROM ztable INTO TABLE @lt_result
  WHERE field = @iv_user_input.
```"

Wartbarkeit

Problem: Magic Numbers

IF lv_status = 3.
  " Verarbeitung
ENDIF.

Review-Kommentar:

“[SUGGESTION] Magic Number 3 ist nicht selbsterklaerend. Bitte Konstante definieren:

CONSTANTS: c_status_approved TYPE i VALUE 3.
IF lv_status = c_status_approved.
```"

Lesbarkeit

Problem: Komplexe Bedingung

IF ( lv_a = 'X' AND lv_b <> 'Y' ) OR ( lv_c = 'Z' AND lv_d > 10 AND lv_e <> '' ).

Review-Kommentar:

“[SUGGESTION] Komplexe Bedingung schwer lesbar. Bitte in aussagekraeftige Hilfsvariablen oder Methode extrahieren:

DATA(lv_is_approved) = xsdbool( lv_a = 'X' AND lv_b <> 'Y' ).
DATA(lv_is_valid) = xsdbool( lv_c = 'Z' AND lv_d > 10 AND lv_e <> '' ).
IF lv_is_approved OR lv_is_valid.
```"

Review-Kultur etablieren

Prinzipien fuer gute Reviews

1. Respektvoll bleiben: Code kritisieren, nicht die Person
2. Lernbereitschaft: Auch als Reviewer offen fuer andere Loesungen
3. Zeitnah reviewen: Reviews nicht aufschieben
4. Gruendlich aber pragmatisch: Nicht jedes Nitpick kommentieren
5. Positives erwaehnen: Nicht nur Kritik, auch Lob

Pair Programming als Alternative

Fuer komplexe Aenderungen kann Pair Programming effektiver sein:

Zusammenfassung

Code Reviews sind eine Investition in die Codequalitaet und Teamkultur. Mit den richtigen Praktiken und Tools werden sie zu einem wertvollen Bestandteil des Entwicklungsprozesses.

Verwandte Themen

• Clean ABAP - Richtlinien fuer wartbaren ABAP-Code
• abapGit - Git-basierte Versionierung und Reviews
• CI/CD mit ABAP Cloud - Automatisierte Pipelines fuer ABAP