Clean ABAP: Wartbaren Code schreiben

" SCHLECHT: Unklare Abkürzungen
DATA: lv_c TYPE i,
      lv_t TYPE string,
      lt_d TYPE TABLE OF mara.

" GUT: Aussagekräftige Namen
DATA: lv_customer_count TYPE i,
      lv_travel_status TYPE string,
      lt_materials TYPE TABLE OF mara.

" SCHLECHT: Unklar was die Methode tut
METHODS process.
METHODS do_it.
METHODS handle.

" GUT: Verb + Objekt beschreibt die Aktion
METHODS calculate_total_price.
METHODS validate_customer_data.
METHODS send_confirmation_email.

" SCHLECHT: Keine Frage erkennbar
DATA lv_status TYPE abap_bool.
METHODS check_customer.

" GUT: Fragen mit is_, has_, can_
DATA lv_is_active TYPE abap_bool.
DATA lv_has_items TYPE abap_bool.
DATA lv_can_be_deleted TYPE abap_bool.

METHODS is_customer_valid RETURNING VALUE(rv_valid) TYPE abap_bool.
METHODS has_open_orders RETURNING VALUE(rv_has_orders) TYPE abap_bool.

" SCHLECHT: Unklare Verantwortung
CLASS zcl_helper DEFINITION.
CLASS zcl_utils DEFINITION.
CLASS zcl_manager DEFINITION.

" GUT: Klare Verantwortung im Namen
CLASS zcl_invoice_calculator DEFINITION.
CLASS zcl_email_sender DEFINITION.
CLASS zcl_customer_validator DEFINITION.

" Interfaces mit IF_ Präfix
INTERFACE zif_payment_provider DEFINITION.
INTERFACE zif_logger DEFINITION.

" SCHLECHT: Methode macht zu viel
METHOD process_order.
  " 1. Validierung (50 Zeilen)
  IF customer_id IS INITIAL.
    " ... Fehlerbehandlung
  ENDIF.
  " ... weitere Validierungen

  " 2. Preisberechnung (80 Zeilen)
  SELECT * FROM pricing_conditions...
  LOOP AT lt_items...
    " ... komplexe Berechnung
  ENDLOOP.

  " 3. Datenbank-Update (40 Zeilen)
  MODIFY ztable FROM TABLE lt_data.
  " ... weitere Updates

  " 4. E-Mail versenden (30 Zeilen)
  " ... E-Mail-Logik
ENDMETHOD.


" GUT: Aufgeteilt in fokussierte Methoden
METHOD process_order.
  validate_order( is_order ).
  DATA(lv_total) = calculate_total_price( is_order-items ).
  save_order( is_order ).
  send_order_confirmation( is_order ).
ENDMETHOD.

METHOD validate_order.
  IF is_order-customer_id IS INITIAL.
    RAISE EXCEPTION TYPE zcx_invalid_order
      EXPORTING textid = zcx_invalid_order=>customer_missing.
  ENDIF.
  " ... weitere Validierungen
ENDMETHOD.

METHOD calculate_total_price.
  rv_total = REDUCE #(
    INIT sum = 0
    FOR item IN it_items
    NEXT sum = sum + item-quantity * item-price
  ).
ENDMETHOD.

" SCHLECHT: Zu viele Parameter
METHODS create_invoice
  IMPORTING
    iv_customer_id   TYPE kunnr
    iv_customer_name TYPE string
    iv_street        TYPE string
    iv_city          TYPE string
    iv_postal_code   TYPE string
    iv_country       TYPE land1
    iv_invoice_date  TYPE dats
    iv_due_date      TYPE dats
    iv_currency      TYPE waers
    iv_payment_terms TYPE string.

" GUT: Struktur verwenden
TYPES: BEGIN OF ty_invoice_data,
         customer    TYPE ty_customer,
         invoice     TYPE ty_invoice_header,
         items       TYPE tt_invoice_items,
       END OF ty_invoice_data.

METHODS create_invoice
  IMPORTING
    is_invoice_data TYPE ty_invoice_data.

" SCHLECHT: EXPORTING für einzelne Werte
METHODS get_customer_name
  IMPORTING iv_customer_id TYPE kunnr
  EXPORTING ev_name        TYPE string.

" Aufruf umständlich
get_customer_name(
  EXPORTING iv_customer_id = lv_id
  IMPORTING ev_name = lv_name
).

" GUT: RETURNING für einzelne Werte
METHODS get_customer_name
  IMPORTING iv_customer_id TYPE kunnr
  RETURNING VALUE(rv_name) TYPE string.

" Aufruf elegant
DATA(lv_name) = get_customer_name( lv_customer_id ).

" SCHLECHT: Doppelte Negation
IF NOT is_invalid = abap_true.
  " ...
ENDIF.

IF NOT has_no_items( ).
  " ...
ENDIF.

" GUT: Positive Formulierung
IF is_valid = abap_true.
  " ...
ENDIF.

IF has_items( ).
  " ...
ENDIF.

" SCHLECHT: Tiefe Verschachtelung
METHOD calculate_discount.
  IF customer IS NOT INITIAL.
    IF customer-is_active = abap_true.
      IF order-total > 1000.
        IF customer-loyalty_level >= 3.
          rv_discount = 15.
        ELSE.
          rv_discount = 10.
        ENDIF.
      ELSE.
        rv_discount = 5.
      ENDIF.
    ENDIF.
  ENDIF.
ENDMETHOD.

" GUT: Guard Clauses (frühe Returns)
METHOD calculate_discount.
  " Vorbedingungen prüfen
  IF customer IS INITIAL.
    RETURN.
  ENDIF.

  IF customer-is_active = abap_false.
    RETURN.
  ENDIF.

  " Hauptlogik - flach und lesbar
  IF order-total > 1000 AND customer-loyalty_level >= 3.
    rv_discount = 15.
    RETURN.
  ENDIF.

  IF order-total > 1000.
    rv_discount = 10.
    RETURN.
  ENDIF.

  rv_discount = 5.
ENDMETHOD.

" SCHLECHT: Lange IF-ELSEIF-Kette
IF lv_status = 'N'.
  lv_text = 'Neu'.
ELSEIF lv_status = 'P'.
  lv_text = 'In Bearbeitung'.
ELSEIF lv_status = 'C'.
  lv_text = 'Abgeschlossen'.
ELSEIF lv_status = 'X'.
  lv_text = 'Storniert'.
ENDIF.

" GUT: CASE ist klarer
lv_text = SWITCH #( lv_status
  WHEN 'N' THEN 'Neu'
  WHEN 'P' THEN 'In Bearbeitung'
  WHEN 'C' THEN 'Abgeschlossen'
  WHEN 'X' THEN 'Storniert'
  ELSE 'Unbekannt'
).

" SCHLECHT: Returncode-basierte Fehlerbehandlung
METHODS validate_order
  IMPORTING is_order TYPE ty_order
  EXPORTING ev_error TYPE string
  RETURNING VALUE(rv_success) TYPE abap_bool.

" Aufruf mit komplizierter Prüfung
IF validate_order(
     EXPORTING is_order = ls_order
     IMPORTING ev_error = lv_error
   ) = abap_false.
  MESSAGE lv_error TYPE 'E'.
ENDIF.


" GUT: Exception-basierte Fehlerbehandlung
METHODS validate_order
  IMPORTING is_order TYPE ty_order
  RAISING   zcx_order_validation.

" Aufruf mit TRY-CATCH
TRY.
    validate_order( ls_order ).
  CATCH zcx_order_validation INTO DATA(lx_error).
    MESSAGE lx_error TYPE 'E'.
ENDTRY.

" Eigene Exception-Klasse mit Nachrichtentexten
CLASS zcx_order_validation DEFINITION
  INHERITING FROM cx_static_check
  CREATE PUBLIC.

  PUBLIC SECTION.
    INTERFACES if_t100_message.

    CONSTANTS:
      BEGIN OF customer_not_found,
        msgid TYPE symsgid VALUE 'ZORDER',
        msgno TYPE symsgno VALUE '001',
        attr1 TYPE scx_attrname VALUE 'CUSTOMER_ID',
        attr2 TYPE scx_attrname VALUE '',
        attr3 TYPE scx_attrname VALUE '',
        attr4 TYPE scx_attrname VALUE '',
      END OF customer_not_found,

      BEGIN OF invalid_quantity,
        msgid TYPE symsgid VALUE 'ZORDER',
        msgno TYPE symsgno VALUE '002',
        attr1 TYPE scx_attrname VALUE 'QUANTITY',
        attr2 TYPE scx_attrname VALUE '',
        attr3 TYPE scx_attrname VALUE '',
        attr4 TYPE scx_attrname VALUE '',
      END OF invalid_quantity.

    DATA customer_id TYPE kunnr READ-ONLY.
    DATA quantity TYPE i READ-ONLY.

    METHODS constructor
      IMPORTING
        textid      LIKE if_t100_message=>t100key OPTIONAL
        previous    TYPE REF TO cx_root OPTIONAL
        customer_id TYPE kunnr OPTIONAL
        quantity    TYPE i OPTIONAL.
ENDCLASS.

" SCHLECHT: Fehler verschlucken
TRY.
    do_something( ).
  CATCH cx_root.
    " Nichts tun - Fehler ignoriert!
ENDTRY.

" SCHLECHT: Alle Fehler gleich behandeln
CATCH cx_root INTO DATA(lx_error).
  WRITE: 'Ein Fehler ist aufgetreten'.

" GUT: Spezifische Fehlerbehandlung
TRY.
    send_email( ls_email ).
  CATCH zcx_email_invalid_address INTO DATA(lx_invalid).
    " Benutzer informieren - behebbar
    MESSAGE lx_invalid TYPE 'W'.
  CATCH zcx_email_server_error INTO DATA(lx_server).
    " Logging für Admin - Retry später
    log_error( lx_server ).
    RAISE EXCEPTION lx_server.
ENDTRY.

" SCHLECHT: Kommentar erklärt offensichtlichen Code
" Addiere 1 zu counter
ADD 1 TO lv_counter.

" Prüfe ob Kunde aktiv ist
IF customer-is_active = abap_true.

" Schleife über alle Items
LOOP AT lt_items INTO DATA(ls_item).


" GUT: Kommentar erklärt WARUM, nicht WAS
" Temporärer Fix bis Release 2.0 - siehe Issue #4711
lv_offset = lv_offset + 1.

" Archivierte Kunden werden separat in Batch-Job verarbeitet
IF customer-is_active = abap_true.

"! <p class="shorttext synchronized">Berechnet den Gesamtpreis inkl. Rabatt</p>
"!
"! @parameter it_items | <p class="shorttext synchronized">Bestellpositionen</p>
"! @parameter iv_discount_percent | <p class="shorttext synchronized">Rabatt in Prozent (0-100)</p>
"! @parameter rv_total | <p class="shorttext synchronized">Gesamtpreis nach Rabatt</p>
"! @raising zcx_calculation | <p class="shorttext synchronized">Bei ungültigen Eingabewerten</p>
METHODS calculate_total
  IMPORTING
    it_items            TYPE tt_order_items
    iv_discount_percent TYPE i DEFAULT 0
  RETURNING
    VALUE(rv_total)     TYPE netwr
  RAISING
    zcx_calculation.

" SCHLECHT: Direkte Abhängigkeit - nicht testbar
CLASS zcl_order_processor DEFINITION.
  PUBLIC SECTION.
    METHODS process_order
      IMPORTING is_order TYPE ty_order.
ENDCLASS.

CLASS zcl_order_processor IMPLEMENTATION.
  METHOD process_order.
    " Direkte Instanzierung - schwer zu testen
    DATA(lo_email_sender) = NEW zcl_email_sender( ).
    lo_email_sender->send( ... ).

    " Direkte DB-Zugriffe - Test verändert echte Daten
    MODIFY ztable FROM ls_data.
  ENDMETHOD.
ENDCLASS.


" GUT: Dependency Injection - testbar
CLASS zcl_order_processor DEFINITION.
  PUBLIC SECTION.
    METHODS constructor
      IMPORTING
        io_email_sender TYPE REF TO zif_email_sender
        io_repository   TYPE REF TO zif_order_repository.

    METHODS process_order
      IMPORTING is_order TYPE ty_order.

  PRIVATE SECTION.
    DATA mo_email_sender TYPE REF TO zif_email_sender.
    DATA mo_repository   TYPE REF TO zif_order_repository.
ENDCLASS.

CLASS zcl_order_processor IMPLEMENTATION.
  METHOD constructor.
    mo_email_sender = io_email_sender.
    mo_repository   = io_repository.
  ENDMETHOD.

  METHOD process_order.
    " Verwendet injizierte Abhängigkeiten
    mo_repository->save( is_order ).
    mo_email_sender->send_confirmation( is_order ).
  ENDMETHOD.
ENDCLASS.

" Test mit Mock-Objekten
CLASS ltc_order_processor DEFINITION
  FOR TESTING
  DURATION SHORT
  RISK LEVEL HARMLESS.

  PRIVATE SECTION.
    DATA mo_cut TYPE REF TO zcl_order_processor.  " Class Under Test
    DATA mo_email_mock TYPE REF TO ltd_email_sender.
    DATA mo_repo_mock TYPE REF TO ltd_order_repository.

    METHODS setup.
    METHODS test_process_sends_email FOR TESTING.
ENDCLASS.

CLASS ltc_order_processor IMPLEMENTATION.
  METHOD setup.
    " Test Doubles erstellen
    mo_email_mock = NEW ltd_email_sender( ).
    mo_repo_mock = NEW ltd_order_repository( ).

    " Class Under Test mit Mocks instanzieren
    mo_cut = NEW zcl_order_processor(
      io_email_sender = mo_email_mock
      io_repository   = mo_repo_mock
    ).
  ENDMETHOD.

  METHOD test_process_sends_email.
    " Given
    DATA(ls_order) = VALUE ty_order(
      order_id = '12345'
      customer_email = '[email protected]'
    ).

    " When
    mo_cut->process_order( ls_order ).

    " Then
    cl_abap_unit_assert=>assert_true(
      act = mo_email_mock->was_send_called
      msg = 'E-Mail sollte versendet werden'
    ).
  ENDMETHOD.
ENDCLASS.

" Test Double für Email Sender
CLASS ltd_email_sender DEFINITION FOR TESTING.
  PUBLIC SECTION.
    INTERFACES zif_email_sender.
    DATA was_send_called TYPE abap_bool.
ENDCLASS.

CLASS ltd_email_sender IMPLEMENTATION.
  METHOD zif_email_sender~send_confirmation.
    was_send_called = abap_true.
  ENDMETHOD.
ENDCLASS.

METHOD calc.
  DATA: lt_d TYPE TABLE OF mara,
        lv_t TYPE p DECIMALS 2,
        lv_c TYPE i.

  SELECT * FROM mara INTO TABLE lt_d
    WHERE matnr IN @s_matnr.

  LOOP AT lt_d INTO DATA(ls_d).
    IF ls_d-mtart = 'FERT' OR ls_d-mtart = 'HALB'.
      SELECT SINGLE netpr FROM a]
        INTO @DATA(lv_p)
        WHERE matnr = @ls_d-matnr.
      IF sy-subrc = 0.
        lv_t = lv_t + lv_p.
        lv_c = lv_c + 1.
      ENDIF.
    ENDIF.
  ENDLOOP.

  IF lv_c > 0.
    rv_result = lv_t / lv_c.
  ENDIF.
ENDMETHOD.

"! <p class="shorttext synchronized">Berechnet Durchschnittspreis für Fertigprodukte</p>
"!
"! @parameter it_material_range | <p class="shorttext synchronized">Materialnummern-Selektion</p>
"! @parameter rv_average_price | <p class="shorttext synchronized">Durchschnittspreis</p>
METHOD calculate_average_price_for_finished_goods.
  DATA(lt_materials) = get_finished_goods( it_material_range ).

  IF lt_materials IS INITIAL.
    RETURN.
  ENDIF.

  DATA(lt_prices) = get_prices_for_materials( lt_materials ).

  rv_average_price = calculate_average( lt_prices ).
ENDMETHOD.


METHOD get_finished_goods.
  SELECT matnr
    FROM mara
    WHERE matnr IN @it_material_range
      AND mtart IN ('FERT', 'HALB')
    INTO TABLE @rt_materials.
ENDMETHOD.


METHOD get_prices_for_materials.
  SELECT matnr, netpr
    FROM a]
    FOR ALL ENTRIES IN @it_materials
    WHERE matnr = @it_materials-matnr
    INTO TABLE @rt_prices.
ENDMETHOD.


METHOD calculate_average.
  CHECK it_prices IS NOT INITIAL.

  DATA(lv_total) = REDUCE netwr(
    INIT sum = CONV netwr( 0 )
    FOR price IN it_prices
    NEXT sum = sum + price-netpr
  ).

  rv_average = lv_total / lines( it_prices ).
ENDMETHOD.