Le non early return

4 réponses

Il y a quelques mois, à l’occasion d’un entretien dans une grande banque d’investissement, mon interlocuteur me parle de qualité de code, de bonnes pratiques et plus spécifiquement du « early return ». Ce chef de projet m’indique alors qu’il interdit son utilisation par ses équipes. Voyant mon air surpris, il m’explique comment/pourquoi il en est arrivé là. Je vais essayer de vous retranscrire les principaux points dans la suite de ce billet. Vous verrez que certains d’entre eux sont très spécifiques au métier de la banque.

Avant d’aller plus loin, prenons un exemple à titre d’illustration. Disons qu’une méthode doit valider un dossier si les trois conditions A, B et C sont remplies. Cela se traduit, logiquement par le code suivant :

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
/**
 * Valide le dossier d'un client par rapport a la valeur passee.
 */
public boolean validerDossierClient(final String value) {

    if (value == null) {
        throw new IllegalArgumentException("bla bla");
    }

    boolean ok = true;

    if (ok) {
        // Ici une traitement complexe sur A
        ok = validerA(value);
    }

    if (ok) {
        // Ici une traitement complexe sur B
        ok = validerB(value);
    }

    if (ok) {
        // Ici une traitement complexe sur C
        ok = validerC(value);
    }

    return ok;
}

Pour faire simple, j’ai juste écris des méthodes bidons pour les validations intermédiaires :

1
2
3
private boolean validerA(final String value) {
    return value.contains("A");
}

De nombreux développeurs vont sauter au plafond en voyant ce code. Ils préféreront mettre en place un « early return » dont le principe repose sur la propriété booléenne suivante :

1
a ET b ET c = !( !a OU !b OU !c )

Plus concrètement, voici ce que la plupart des développeur auraient écrit :

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
public boolean validerDossierClient(final String value) {

    if (value == null) {
        throw new IllegalArgumentException("bla bla");
    }

    // Ici une traitement complexe sur A
    boolean ok = validerA(value);

    if (!ok) {
        return false;
    }

    // Ici une traitement complexe sur B
    ok = validerB(value);

    if (!ok) {
        return false;
    }

    // Ici une traitement complexe sur C
    ok = validerC(value);

    if (!ok) {
        return false;
    }

    return true;
}

Dans cette seconde version, le principe est de sortir dès qu’au moins une des validation échoue. On voit d’ailleurs un différence dans le return final. C’est plus simple, plus efficace et plus rapide.

Alors pourquoi se l’interdire ? Tout simplement parce que les développements réalisés pour ce projet sont fait en Inde. Les indiens travaillent très bien. Ce n’est pas mon promos. En revanche, ils sont loin. Quand ils livrent, les passes de vérification de la qualité du code sont longues et difficiles. Et il est fort possible que les spécifications soient déjà différentes (disons obsolètes)… Il se peut donc qu’une conditions n’ait pas été prise en compte. Cela ne se voit pas forcément très bien dans l’exemple, sans doute trop simple. Toujours est-il que la banque préfère perdre un prospect qu’accepter un dossier bancal. La banque ne veut prendre aucun risque, ce qui est d’autant plus vrai dans le contexte économique moribond actuel. Notez que dans d’autres secteurs, comme dans la grande distribution, on aura tendance à faire le choix inverse, càd gagner des clients quitte à subir un taux de satisfaction moindre…

Bien entendu, dans un monde idéal, où les développeurs respecterait scrupuleusement le cahier des charges et dans lequel il n’y aurait pas de bug, ce serait différent. Vous me direz aussi qu’il y a des tests unitaires pour vérifier que tout fonctionne. Sur ce point, mon interlocuteur est d’accord. Il m’explique néanmoins que ses prestataires indiens savent faire en sorte que les tests unitaires passent bien au vert, sans que cela ne prouve vraiment que le programme est conforme. Quelques mauvaises expériences l’ont d’ailleurs bien refroidi…

Et vous, qu’en pensez-vous ?

4 réflexions au sujet de « Le non early return »

  1. Avatar de floppy22floppy22

    Je m’accorde à Lunatix sur la règle de l’unicité du mot clef return dans toutes fonctions / méthodes mais sans condition de nombre de lignes.
    Bien que développeur Java et partiellement .Net, je reste adepte du développement C/C++ dans sa rigueur.
    Un unique return final ne dégrade aucunement les performances (Les tests conditionnels étant correctement réalisés en amont et n’influençant pas le code assembleur sous-adjacent) mais peut au contraire les améliorer de par le traitement de toutes les opérations préalablement réalisées et la libération des ressources intermédiaires « oubliées » et laissées à charge du GC (Java ou .Net).
    Je suis convaincu de la mauvaise habitude des développeurs quant à la déclaration des variables / variables membres / attributs (natifs, objets) au fil de l’eau et des retours au gré des envies.
    Je reste convaincu de la déclaration des varibales / variables membres / attributs (natifs, objets) en tête de fonction(s) / méthode(s) et de l’utilisation du mot clef return toujours en fin de fonction / méthode.
    Seule et unique réponse à la lisibilité / maintenabilité.
    L’early return est maintenu même dans votre exemple …

    ↓

Laisser un commentaire