Clean ABAP : Écrire du code maintenable

" MAUVAIS : Abréviations peu claires
DATA: lv_c TYPE i,
      lv_t TYPE string,
      lt_d TYPE TABLE OF mara.

" BON : Noms significatifs
DATA: lv_customer_count TYPE i,
      lv_travel_status TYPE string,
      lt_materials TYPE TABLE OF mara.

" MAUVAIS : Peu clair ce que fait la méthode
METHODS process.
METHODS do_it.
METHODS handle.

" BON : Verbe + objet décrit l'action
METHODS calculate_total_price.
METHODS validate_customer_data.
METHODS send_confirmation_email.

" MAUVAIS : Aucune question reconnaissable
DATA lv_status TYPE abap_bool.
METHODS check_customer.

" BON : Questions avec 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.

" MAUVAIS : Responsabilité peu claire
CLASS zcl_helper DEFINITION.
CLASS zcl_utils DEFINITION.
CLASS zcl_manager DEFINITION.

" BON : Responsabilité claire dans le nom
CLASS zcl_invoice_calculator DEFINITION.
CLASS zcl_email_sender DEFINITION.
CLASS zcl_customer_validator DEFINITION.

" Interfaces avec préfixe IF_
INTERFACE zif_payment_provider DEFINITION.
INTERFACE zif_logger DEFINITION.

" MAUVAIS : La méthode fait trop de choses
METHOD process_order.
  " 1. Validation (50 lignes)
  IF customer_id IS INITIAL.
    " ... gestion d'erreur
  ENDIF.
  " ... autres validations

  " 2. Calcul de prix (80 lignes)
  SELECT * FROM pricing_conditions...
  LOOP AT lt_items...
    " ... calcul complexe
  ENDLOOP.

  " 3. Mise à jour base de données (40 lignes)
  MODIFY ztable FROM TABLE lt_data.
  " ... autres mises à jour

  " 4. Envoi email (30 lignes)
  " ... logique email
ENDMETHOD.


" BON : Divisé en méthodes focalisées
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.
  " ... autres validations
ENDMETHOD.

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

" MAUVAIS : Trop de paramètres
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.

" BON : Utiliser une structure
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.

" MAUVAIS : EXPORTING pour valeurs uniques
METHODS get_customer_name
  IMPORTING iv_customer_id TYPE kunnr
  EXPORTING ev_name        TYPE string.

" Appel compliqué
get_customer_name(
  EXPORTING iv_customer_id = lv_id
  IMPORTING ev_name = lv_name
).

" BON : RETURNING pour valeurs uniques
METHODS get_customer_name
  IMPORTING iv_customer_id TYPE kunnr
  RETURNING VALUE(rv_name) TYPE string.

" Appel élégant
DATA(lv_name) = get_customer_name( lv_customer_id ).

" MAUVAIS : Double négation
IF NOT is_invalid = abap_true.
  " ...
ENDIF.

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

" BON : Formulation positive
IF is_valid = abap_true.
  " ...
ENDIF.

IF has_items( ).
  " ...
ENDIF.

" MAUVAIS : Imbrication profonde
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.

" BON : Guard Clauses (returns précoces)
METHOD calculate_discount.
  " Vérifier les préconditions
  IF customer IS INITIAL.
    RETURN.
  ENDIF.

  IF customer-is_active = abap_false.
    RETURN.
  ENDIF.

  " Logique principale - plate et lisible
  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.

" MAUVAIS : Longue chaîne IF-ELSEIF
IF lv_status = 'N'.
  lv_text = 'Nouveau'.
ELSEIF lv_status = 'P'.
  lv_text = 'En cours'.
ELSEIF lv_status = 'C'.
  lv_text = 'Terminé'.
ELSEIF lv_status = 'X'.
  lv_text = 'Annulé'.
ENDIF.

" BON : CASE est plus clair
lv_text = SWITCH #( lv_status
  WHEN 'N' THEN 'Nouveau"
  WHEN 'P' THEN 'En cours"
  WHEN 'C' THEN 'Terminé"
  WHEN 'X' THEN 'Annulé"
  ELSE 'Inconnu"
).

" MAUVAIS : Gestion d'erreur basée sur code retour
METHODS validate_order
  IMPORTING is_order TYPE ty_order
  EXPORTING ev_error TYPE string
  RETURNING VALUE(rv_success) TYPE abap_bool.

" Appel avec vérification compliquée
IF validate_order(
     EXPORTING is_order = ls_order
     IMPORTING ev_error = lv_error
   ) = abap_false.
  MESSAGE lv_error TYPE 'E'.
ENDIF.


" BON : Gestion d'erreur basée sur exceptions
METHODS validate_order
  IMPORTING is_order TYPE ty_order
  RAISING   zcx_order_validation.

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

" Classe d'exception propre avec textes de message
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.

" MAUVAIS : Avaler les erreurs
TRY.
    do_something( ).
  CATCH cx_root.
    " Ne rien faire - erreur ignorée !
ENDTRY.

" MAUVAIS : Traiter toutes les erreurs de la même manière
CATCH cx_root INTO DATA(lx_error).
  WRITE: 'Une erreur s''est produite'.

" BON : Traitement d'erreur spécifique
TRY.
    send_email( ls_email ).
  CATCH zcx_email_invalid_address INTO DATA(lx_invalid).
    " Informer l'utilisateur - réparable
    MESSAGE lx_invalid TYPE 'W'.
  CATCH zcx_email_server_error INTO DATA(lx_server).
    " Logging pour admin - Retry plus tard
    log_error( lx_server ).
    RAISE EXCEPTION lx_server.
ENDTRY.

" MAUVAIS : Commentaire explique du code évident
" Ajouter 1 au compteur
ADD 1 TO lv_counter.

" Vérifier si le client est actif
IF customer-is_active = abap_true.

" Boucle sur tous les items
LOOP AT lt_items INTO DATA(ls_item).


" BON : Commentaire explique POURQUOI, pas QUOI
" Correctif temporaire jusqu'à la release 2.0 - voir Issue #4711
lv_offset = lv_offset + 1.

" Les clients archivés sont traités séparément dans un job batch
IF customer-is_active = abap_true.

"! <p class="shorttext synchronized">Calcule le prix total avec remise</p>
"!
"! @parameter it_items | <p class="shorttext synchronized">Positions de commande</p>
"! @parameter iv_discount_percent | <p class="shorttext synchronized">Remise en pourcentage (0-100)</p>
"! @parameter rv_total | <p class="shorttext synchronized">Prix total après remise</p>
"! @raising zcx_calculation | <p class="shorttext synchronized">En cas de valeurs d'entrée invalides</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.

" MAUVAIS : Dépendance directe - non testable
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.
    " Instanciation directe - difficile à tester
    DATA(lo_email_sender) = NEW zcl_email_sender( ).
    lo_email_sender->send( ... ).

    " Accès DB directs - le test modifie les vraies données
    MODIFY ztable FROM ls_data.
  ENDMETHOD.
ENDCLASS.


" BON : Injection de dépendances - testable
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.
    " Utilise les dépendances injectées
    mo_repository->save( is_order ).
    mo_email_sender->send_confirmation( is_order ).
  ENDMETHOD.
ENDCLASS.

" Test avec objets Mock
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.
    " Créer Test Doubles
    mo_email_mock = NEW ltd_email_sender( ).
    mo_repo_mock = NEW ltd_order_repository( ).

    " Instancier Class Under Test avec Mocks
    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 = 'L''email devrait être envoyé"
    ).
  ENDMETHOD.
ENDCLASS.

" Test Double pour 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">Calcule le prix moyen pour produits finis</p>
"!
"! @parameter it_material_range | <p class="shorttext synchronized">Sélection de numéros de matières</p>
"! @parameter rv_average_price | <p class="shorttext synchronized">Prix moyen</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.