Clean ABAP: Escribir codigo mantenible

" MAL: Abreviaturas poco claras
DATA: lv_c TYPE i,
      lv_t TYPE string,
      lt_d TYPE TABLE OF mara.

" BIEN: Nombres descriptivos
DATA: lv_customer_count TYPE i,
      lv_travel_status TYPE string,
      lt_materials TYPE TABLE OF mara.

" MAL: No esta claro que hace el metodo
METHODS process.
METHODS do_it.
METHODS handle.

" BIEN: Verbo + objeto describe la accion
METHODS calculate_total_price.
METHODS validate_customer_data.
METHODS send_confirmation_email.

" MAL: No se reconoce como pregunta
DATA lv_status TYPE abap_bool.
METHODS check_customer.

" BIEN: Preguntas con 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.

" MAL: Responsabilidad poco clara
CLASS zcl_helper DEFINITION.
CLASS zcl_utils DEFINITION.
CLASS zcl_manager DEFINITION.

" BIEN: Responsabilidad clara en el nombre
CLASS zcl_invoice_calculator DEFINITION.
CLASS zcl_email_sender DEFINITION.
CLASS zcl_customer_validator DEFINITION.

" Interfaces con prefijo IF_
INTERFACE zif_payment_provider DEFINITION.
INTERFACE zif_logger DEFINITION.

" MAL: El metodo hace demasiado
METHOD process_order.
  " 1. Validacion (50 lineas)
  IF customer_id IS INITIAL.
    " ... manejo de errores
  ENDIF.
  " ... mas validaciones

  " 2. Calculo de precio (80 lineas)
  SELECT * FROM pricing_conditions...
  LOOP AT lt_items...
    " ... calculo complejo
  ENDLOOP.

  " 3. Actualizacion de base de datos (40 lineas)
  MODIFY ztable FROM TABLE lt_data.
  " ... mas actualizaciones

  " 4. Enviar email (30 lineas)
  " ... logica de email
ENDMETHOD.


" BIEN: Dividido en metodos enfocados
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.
  " ... mas validaciones
ENDMETHOD.

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

" MAL: Demasiados parametros
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.

" BIEN: Usar estructura
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.

" MAL: EXPORTING para valores individuales
METHODS get_customer_name
  IMPORTING iv_customer_id TYPE kunnr
  EXPORTING ev_name        TYPE string.

" Llamada incomoda
get_customer_name(
  EXPORTING iv_customer_id = lv_id
  IMPORTING ev_name = lv_name
).

" BIEN: RETURNING para valores individuales
METHODS get_customer_name
  IMPORTING iv_customer_id TYPE kunnr
  RETURNING VALUE(rv_name) TYPE string.

" Llamada elegante
DATA(lv_name) = get_customer_name( lv_customer_id ).

" MAL: Doble negacion
IF NOT is_invalid = abap_true.
  " ...
ENDIF.

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

" BIEN: Formulacion positiva
IF is_valid = abap_true.
  " ...
ENDIF.

IF has_items( ).
  " ...
ENDIF.

" MAL: Anidamiento profundo
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.

" BIEN: Guard Clauses (retornos tempranos)
METHOD calculate_discount.
  " Verificar precondiciones
  IF customer IS INITIAL.
    RETURN.
  ENDIF.

  IF customer-is_active = abap_false.
    RETURN.
  ENDIF.

  " Logica principal - plana y legible
  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.

" MAL: Cadena larga IF-ELSEIF
IF lv_status = 'N'.
  lv_text = 'Nuevo'.
ELSEIF lv_status = 'P'.
  lv_text = 'En proceso'.
ELSEIF lv_status = 'C'.
  lv_text = 'Completado'.
ELSEIF lv_status = 'X'.
  lv_text = 'Cancelado'.
ENDIF.

" BIEN: CASE es mas claro
lv_text = SWITCH #( lv_status
  WHEN 'N' THEN 'Nuevo'
  WHEN 'P' THEN 'En proceso'
  WHEN 'C' THEN 'Completado'
  WHEN 'X' THEN 'Cancelado'
  ELSE 'Desconocido'
).

" MAL: Manejo de errores basado en codigo de retorno
METHODS validate_order
  IMPORTING is_order TYPE ty_order
  EXPORTING ev_error TYPE string
  RETURNING VALUE(rv_success) TYPE abap_bool.

" Llamada con verificacion complicada
IF validate_order(
     EXPORTING is_order = ls_order
     IMPORTING ev_error = lv_error
   ) = abap_false.
  MESSAGE lv_error TYPE 'E'.
ENDIF.


" BIEN: Manejo de errores basado en excepciones
METHODS validate_order
  IMPORTING is_order TYPE ty_order
  RAISING   zcx_order_validation.

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

" Clase de excepcion propia con textos de mensaje
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.

" MAL: Tragarse el error
TRY.
    do_something( ).
  CATCH cx_root.
    " No hacer nada - error ignorado!
ENDTRY.

" MAL: Tratar todos los errores igual
CATCH cx_root INTO DATA(lx_error).
  WRITE: 'Ha ocurrido un error'.

" BIEN: Manejo de errores especifico
TRY.
    send_email( ls_email ).
  CATCH zcx_email_invalid_address INTO DATA(lx_invalid).
    " Informar al usuario - corregible
    MESSAGE lx_invalid TYPE 'W'.
  CATCH zcx_email_server_error INTO DATA(lx_server).
    " Logging para admin - reintentar mas tarde
    log_error( lx_server ).
    RAISE EXCEPTION lx_server.
ENDTRY.

" MAL: Comentario explica codigo obvio
" Agregar 1 al contador
ADD 1 TO lv_counter.

" Verificar si el cliente esta activo
IF customer-is_active = abap_true.

" Bucle sobre todos los items
LOOP AT lt_items INTO DATA(ls_item).


" BIEN: Comentario explica POR QUE, no QUE
" Fix temporal hasta Release 2.0 - ver Issue #4711
lv_offset = lv_offset + 1.

" Clientes archivados se procesan separadamente en job batch
IF customer-is_active = abap_true.

"! <p class="shorttext synchronized">Calcula el precio total incl. descuento</p>
"!
"! @parameter it_items | <p class="shorttext synchronized">Posiciones del pedido</p>
"! @parameter iv_discount_percent | <p class="shorttext synchronized">Descuento en porcentaje (0-100)</p>
"! @parameter rv_total | <p class="shorttext synchronized">Precio total despues del descuento</p>
"! @raising zcx_calculation | <p class="shorttext synchronized">Con valores de entrada invalidos</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.

" MAL: Dependencia directa - no testeable
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.
    " Instanciacion directa - dificil de testear
    DATA(lo_email_sender) = NEW zcl_email_sender( ).
    lo_email_sender->send( ... ).

    " Acceso directo a BD - test modifica datos reales
    MODIFY ztable FROM ls_data.
  ENDMETHOD.
ENDCLASS.


" BIEN: Dependency Injection - testeable
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.
    " Usa dependencias inyectadas
    mo_repository->save( is_order ).
    mo_email_sender->send_confirmation( is_order ).
  ENDMETHOD.
ENDCLASS.

" Test con objetos 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.
    " Crear Test Doubles
    mo_email_mock = NEW ltd_email_sender( ).
    mo_repo_mock = NEW ltd_order_repository( ).

    " Instanciar Class Under Test con 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 = 'Email deberia ser enviado'
    ).
  ENDMETHOD.
ENDCLASS.

" Test Double para 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">Calcula precio promedio para productos terminados</p>
"!
"! @parameter it_material_range | <p class="shorttext synchronized">Seleccion de numeros de material</p>
"! @parameter rv_average_price | <p class="shorttext synchronized">Precio promedio</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.