Bonnes pratiques de revue de code pour ABAP

Les revues de code sont un élément essentiel du développement logiciel professionnel. Dans le monde ABAP, elles gagnent en importance grâce aux outils modernes comme abapGit et l’intégration ADT. Cet article montre comment concevoir des revues de code efficaces tout en améliorant la qualité du code et la culture d’équipe.

La valeur des revues de code

Les revues de code apportent bien plus que la simple recherche de bugs :

Statistiques sur les revues de code

Les études montrent systématiquement la valeur des revues de code :

• Les revues de code trouvent 60-70% des défauts avant les tests
• Sans revues, environ 15 erreurs pour 1000 lignes de code arrivent en production
• Avec les revues, ce nombre tombe à environ 5 erreurs
• La détection précoce des erreurs génère des économies considérables par rapport aux corrections tardives

Checklist pour les revues de code ABAP

Une checklist structurée aide à effectuer des revues cohérentes.

1. Fonctionnalité

• Le code répond-il aux exigences ?
• Tous les critères d’acceptation sont-ils couverts ?
• Les cas limites ont-ils été pris en compte ?
• La gestion des erreurs est-elle complète ?

2. Qualité du code

• Les noms sont-ils parlants (variables, méthodes, classes) ?
• Les méthodes sont-elles courtes et focalisées (Single Responsibility) ?
• Y a-t-il de la duplication de code (principe DRY) ?
• Le code est-il lisible sans beaucoup de commentaires ?

3. Spécifique ABAP

• Les constructions ABAP modernes sont-elles utilisées ?
• Les instructions SELECT sont-elles performantes (index, agrégations) ?
• Le travail sur les tables internes est-il efficace ?
• Les Authority Checks sont-ils présents là où nécessaire ?

4. Sécurité

• Pas de possibilités d’injection SQL ?
• Authority Checks pour les opérations sensibles ?
• Pas de credentials codés en dur ?
• Validation des entrées complète ?

5. Testabilité

• Y a-t-il des tests ABAP Unit ?
• Les tests sont-ils significatifs ?
• Le code est-il structuré de manière testable (Dependency Injection) ?
• Des Test-Doubles sont-ils présents pour les dépendances externes ?

Règles Clean ABAP pour les revues

Lors des revues de code, les Clean ABAP Guidelines doivent être vérifiées systématiquement.

Vérifier les conventions de nommage

" Commentaire de revue : Nom trop générique
DATA: lv_val TYPE i.

" Mieux : Nom parlant
DATA: lv_total_amount TYPE i.

Commentaire de revue typique :

“Le nom de variable lv_val n’est pas parlant. Qu’est-ce qui est stocké ici ? Veuillez utiliser un nom descriptif comme lv_order_count ou lv_total_amount.”

Vérifier la longueur des méthodes

" Commentaire de revue : Méthode trop longue, plusieurs responsabilités
METHOD process_order.
  " 200 lignes de code...
  " Validation
  " Calcul
  " Accès base de données
  " Sortie
ENDMETHOD.

Commentaire de revue typique :

“Cette méthode a plus de 200 lignes et plusieurs responsabilités. Veuillez la diviser en méthodes plus petites : validate_order(), calculate_totals(), save_to_database(), prepare_output().”

Éviter les paramètres booléens

" Commentaire de revue : Paramètre booléen peu clair
CALL METHOD process_data( iv_flag = abap_true ).

Commentaire de revue typique :

“Les paramètres booléens sont difficiles à lire. Que signifie iv_flag = abap_true ? Mieux : des méthodes séparées comme process_data_in_test_mode() ou utiliser un type enum.”

Exceptions plutôt que codes retour

" Commentaire de revue : Code retour au lieu d'exception
METHOD get_customer.
  IF customer_not_found.
    rv_success = abap_false.
    RETURN.
  ENDIF.
ENDMETHOD.

Commentaire de revue typique :

“Veuillez utiliser des exceptions au lieu de codes retour. Les exceptions rendent le cas d’erreur explicite et forcent le traitement. Exemple : RAISE EXCEPTION TYPE cx_customer_not_found.”

Utiliser la syntaxe moderne

" Commentaire de revue : Syntaxe obsolète
DATA: lt_orders TYPE TABLE OF zorder.
CLEAR lt_orders.
REFRESH lt_orders.
MOVE wa_order TO lt_orders.
APPEND lt_orders.

" Recommandé : Syntaxe moderne
DATA(lt_orders) = VALUE zorder_tt( ).
APPEND wa_order TO lt_orders.

Commentaire de revue typique :

“Veuillez utiliser la syntaxe ABAP moderne. MOVE ... TO et REFRESH sont obsolètes. Les déclarations inline avec DATA() et le constructeur VALUE sont plus lisibles.”

Donner un feedback constructif

La façon dont le feedback est donné est cruciale pour la dynamique d’équipe.

À faire pour le feedback

À ne pas faire pour le feedback

Exemples de bons commentaires de revue

Au lieu de :

“C’est faux, on ne fait pas comme ça.”

Mieux :

“Cette solution fonctionne, mais je vois un risque avec de grands volumes de données. As-tu envisagé de faire le traitement par paquets ? Exemple : LOOP AT lt_data ASSIGNING FIELD-SYMBOL(<ls_data>) PACKAGE SIZE 1000.”

Au lieu de :

“Mauvais nommage.”

Mieux :

“Le nom de méthode do_it() ne révèle pas ce que fait la méthode. Que dirais-tu de calculate_order_total() ou send_notification() ?”

Catégories de feedback

Utilisez des labels pour signaler l’importance :

Outils de revue de code pour ABAP

ADT (ABAP Development Tools)

ADT dans Eclipse offre un support de revue de base :

Fonctions de comparaison :

• Comparaison d’objets entre systèmes (Development vs. Quality)
• Historique des versions dans la gestion des transports
• Historique des modifications locales

Navigation :

• Call Hierarchy pour l’analyse d’impact
• Where-Used-Lists pour les effets des modifications
• Suggestions Quick-Fix pour les problèmes courants

Intégration ATC :

• ABAP Test Cockpit directement dans l’IDE
• Retour immédiat sur la qualité du code
• Checks personnalisés définissables

abapGit pour les revues de code

abapGit permet des revues modernes basées sur les Pull Requests :

Workflow avec abapGit :

1. Créer une branche feature

   Repository > Branch > Create Branch

2. Développer et committer le code

   Repository > Stage > Commit

3. Push vers le repository distant

   Repository > Push

4. Créer une Pull Request (sur GitHub/GitLab)

   • Description des modifications
   • Assigner des reviewers
   • Ajouter des labels

5. Effectuer la revue

   • Utiliser la vue diff
   • Écrire des commentaires inline
   • Approve/Request Changes

6. Merge après approbation

   • Squash Merge pour un historique propre
   • Supprimer la branche

Avantages des revues abapGit :

Outils SAP natifs

ABAP Test Cockpit (ATC) :

• Analyse statique automatique du code
• Intégrable dans le processus de revue
• Checks personnalisés possibles

Transport Management :

• Workflow pour les validations
• Principe des quatre yeux
• Documentation des modifications

Intégrer la revue de code dans le CI/CD

Les pipelines CI/CD peuvent automatiser et sécuriser les processus de revue.

Checks automatiques avant la revue

# Exemple : Workflow GitHub Actions
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

Règles de protection de branche

Configurez la protection de branche pour les revues obligatoires :

Métriques de revue

Mesurez l’efficacité de votre processus de revue :

Commentaires de revue typiques avec exemples

Performance

Problème : SELECT imbriqué dans une boucle

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

Commentaire de revue :

“[BLOCKING] SELECT dans LOOP cause le problème N+1 Query. Veuillez collecter tous les Customer-IDs et les interroger avec un seul SELECT avec FOR ALL ENTRIES :

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

Puis accéder avec READ TABLE dans la boucle.”

Gestion des erreurs

Problème : Bloc CATCH vide

TRY.
    process_critical_data( ).
  CATCH cx_root.
    " Ne rien faire
ENDTRY.

Commentaire de revue :

“[BLOCKING] Le bloc CATCH vide avale les erreurs. Veuillez soit :

• Logger l’exception : cl_demo_output=>write( cx_root->get_text( ) )
• Relancer l’exception : RAISE EXCEPTION cx_...
• Ignorer consciemment avec commentaire : " L'erreur est ignorée parce que ...”

Sécurité

Problème : SQL dynamique

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

Commentaire de revue :

“[BLOCKING] Danger d’injection SQL ! L’entrée utilisateur iv_user_input est utilisée directement dans le SQL. Veuillez utiliser la liaison de paramètres :

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

Maintenabilité

Problème : Nombres magiques

IF lv_status = 3.
  " Traitement
ENDIF.

Commentaire de revue :

“[SUGGESTION] Le nombre magique 3 n’est pas explicite. Veuillez définir une constante :

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

Lisibilité

Problème : Condition complexe

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

Commentaire de revue :

“[SUGGESTION] Condition complexe difficile à lire. Veuillez extraire dans des variables d’aide parlantes ou une méthode :

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.
```"

Établir une culture de revue

Principes pour de bonnes revues

1. Rester respectueux : Critiquer le code, pas la personne
2. Ouverture à l’apprentissage : Être ouvert à d’autres solutions même en tant que reviewer
3. Reviewer rapidement : Ne pas reporter les revues
4. Rigoureux mais pragmatique : Ne pas commenter chaque détail
5. Mentionner le positif : Pas seulement des critiques, aussi des éloges

Le Pair Programming comme alternative

Pour les modifications complexes, le pair programming peut être plus efficace :

Résumé

Les revues de code sont un investissement dans la qualité du code et la culture d’équipe. Avec les bonnes pratiques et les bons outils, elles deviennent une partie précieuse du processus de développement.

Sujets connexes

• Clean ABAP - Directives pour un code ABAP maintenable
• abapGit - Versioning basé sur Git et revues
• CI/CD avec ABAP Cloud - Pipelines automatisés pour ABAP