Blog Arolla

L’expressivité du fonctionnel avec Java 8

Contexte

Dans ma dernière mission chez un fournisseur de voyage, l'application a été migrée en Java 8.

Pour moi qui utilise Guava depuis quelques années (depuis cet article de Cyrille Martraire en fait), c'est une excellente nouvelle.
J'avais constaté qu'en de multiples endroits, on parcourait des listes, puis en fonction de l'item, on remplissait une nouvelle liste, ou alors on filtrait, etc .... Bref, une sorte d'énorme bac à sable pour tester la programmation fonctionnelle et son expressivité. J'étais ravi.

Et un jour ... en cherchant l'origine d'un bug ... je tombe sur ce bout de code :

/**
* Compute how many chunks to delete for
* given Travel to insert in database.
* The number of chunks is mapped with fares to exclude from billing
*/
private int compute(final Travel travel) {
    int chunksToRemove = 0;
    final Price[] prices = travel.getPrices();
    if (!ArrayUtils.isEmpty(prices)) {
        Arrays.asList(prices);
        for (final Price price : prices) {
            final FarePolicy[] farePolicies = price.getFarePolicies();
            final CustomerFare[] customerFares = price.getCustomerFares();
            for (final FarePolicy fareCalculation : farePolicies) {
                for (final String excludedFare : billing.getExcludedFares()) {
                    if (excludedFare.equals(fareCalculation.getFareName())) {
                        for (final CustomerFare customerFare : customerFares) {
                            if (customerFare.getType().equals(
                                fareCalculation.getPassengerType())) {
                                chunksToRemove +=
                                        (fareCalculation.getChunks().length
                                        * customerFare.getCustomersQuantity());
                            }
                        }
                    }
                }
            }
        }
    }
    return chunksToRemove;
}

Bon là, je crois que c'est urgent, il faut refactorer !!!
Avant de modifier le code, commençons par une rapide analyse du code.

Premiers constats

  • Aïe, on est dans une méthode privée.
  • Arrays.asList(prices); ne sert à rien
  • Si billing.getExcludedFares() est null, alors on aura une NullPointerException dans notre boucle for. C'est d'ailleurs le bug qui m'a amené ici. (Ce n'est pas le cas pour farePolicies et customerFares car l'API qui nous fournit ces objets nous assure que ces tableaux ne seront jamais nuls)
  • Le commentaire et le nom de la méthode ne sont pas d'une grande aide pour comprendre ce que fait cette méthode.

Cette méthode est appelée lors de l'application d'un tarif sur un voyage. Elle calcule un nombre utilisé lors de l'application du tarif. Son importance et sa complexité justifient selon moi la création d'un objet dédié.

Pour pouvoir commencer à refactorer, je dois pouvoir être capable de tester simplement le code existant, afin d'être sûr de ne pas introduire de régression. Or l'intention de cette méthode privée est "testée" via sa méthode appelante. Cool j'ai un jeu de test :+1:. Je vais donc l'extraire et l'utiliser pour tester mon nouvel objet.

Premières corrections

  • Créer une classe propre pour cette méthode et isoler le test.
  • Supprimer le code inutile
  • Vérifier au début de la méthode si toutes mes données sont cohérentes avant de continuer le calcul.
  • Supprimer le commentaire et trouver un nom adéquat quand j'aurais compris son utilité/fonctionnement
  • Extraire une méthode qui va effectuer le calcul pour un Price

Et ça donne ça :

public int compute(final Travel travel) {
    if (!acceptsPreconditions(travel)) {
        return DEFAULT;
    }

    final Price[] prices = travel.getPrices();
    int chunkToExclude = DEFAULT;
    for (final Price price : prices) {
        chunkToExclude += countExclusionsForPrice( price);
    }
    return chunkToExclude;
}

public int countExclusionsForPrice(Price price) {
    int retour = DEFAULT;
    final FarePolicy[] farePolicies = price.getFarePolicies();
    final CustomerFare[] customerFares = price.getCustomerFares();
    for (final FarePolicy farePolicy : farePolicies) {
        for (final FareLabel excludedFare : billing.getExcludedFares()) {
            if (excludedFare.equals(farePolicy.getFareName())) {
                for (final CustomerFare customerFare : customerFares) {
                    if (customerFare.getType().equals(farePolicy.getCustomerType())) {
                        retour += 
                               farePolicy.getChunks().length 
                               * customerFare.getCustomersQuantity();
                    }
                }
            }
        }
    }
    return retour;
}

Avec une constante DEFAULT = 0 et une méthode acceptPreconditions(...) qui vérifie que le Travel passé en paramètre est bon mais aussi que billing est dans un état cohérent avec le calcul. Cette méthode pourra être enrichie avec toutes les futures éventuelles vérifications préalables.

Voilà, c'est un peu mieux mais sans programmation fonctionnelle, on peut difficilement aller plus loin. On ne peut pas séparer les deux boucles imbriquées. C'est dommage pour la lisibilité du code.

On peut maintenant rentrer dans la phase 2 et essayer de comprendre en profondeur ce code.

Deuxième phase

  • On a un Travel qui contient des Prices
  • Chaque Price contient
    • des FarePolicy
    • des CustomerFare
  • Chaque FarePolicy contient
    • un FareLabel
    • un CustomerType
    • les Chunks (sorte de bout de trajet) du Travel associés
  • Chaque CustomerFare contient
    • un CustomerType
    • le nombre de passagers du Travel associés
  • On a un billing qui retourne les FareLabel exclus.

Lorsque l'on calcule le tarif global du Travel, on applique un tarif à chaque morceau de trajet (Chunk).
Le nombre de Chunk sur lesquels on applique un tarif est multiplié par le nombre de passagers concernés par cette politique tarifaire.

Pour avoir le tarif total, il ne faut pas comptabiliser les Chunk correspondant aux tarifs exclus. C'est donc le but de notre méthode : compter le nombre de Chunk a ne pas comptabiliser lors de la tarification.

Voici le code de test extrait de la méthode originale. Pour aider à comprendre, il faut passer un peu de temps sur la méthode prepareTavel() qui crée un Travel avec ses morceaux, ses tarifs, ses passagers :

@InjectMocks
private ChunksFromTravelBillingExcluder sut; // System Under Test
@Mock
private Billing billing;

/**
 * nom de méthode de test pourri
 * mais je ne comprends pas encore exactement comment ca fonctionne
 */
@Test
public void should_compute() {
    // GIVEN
    final Travel travel = prepareTravel();

    // WHEN
    when(billing.getExcludedFares()).thenReturn(
            new FareLabel[]{
                 FareLabel.of("EXCLUDED_FARE"), 
                 FareLabel.of("EXCLUDED_FARE_2")
            });

    final int actualQuantity = sut.count(travel);

    // THEN
    assertEquals(18, actualQuantity);
}


private Travel prepareTravel() {

    final FarePolicy farePolicy1 = new FarePolicy();
    farePolicy1.setCustomerType(CustomerType.of("CUSTOMER_TYPE_1"));
    farePolicy1.setChunks(new Chunk[]{new Chunk(), new Chunk()});
    final FareLabel fare1 = FareLabel.of("FARE_1");
    farePolicy1.setFareLabel(fare1);

    final FarePolicy farePolicy2 = new FarePolicy();
    farePolicy2.setCustomerType(CustomerType.of("CUSTOMER_TYPE_2"));
    farePolicy2.setChunks(new Chunk[]{new Chunk(), new Chunk(), new Chunk()});
    farePolicy2.setFareLabel(FareLabel.of("EXCLUDED_FARE"));

    final CustomerFare customerFare1 = new CustomerFare();
    customerFare1.setCustomerType(CustomerType.of("CUSTOMER_TYPE_1"));
    customerFare1.setCustomersQuantity(2);

    final CustomerFare customerFare2 = new CustomerFare();
    customerFare2.setCustomerType(CustomerType.of("CUSTOMER_TYPE_2"));
    customerFare2.setCustomersQuantity(3);

    final Price price = new Price();
    price.setFarePolicies(new FarePolicy[]{farePolicy1, farePolicy2});
    price.setCustomerFares(new CustomerFare[]{customerFare1, customerFare2});

    final Travel travel = new Travel();
    travel.setChunks(new Chunk[]{new Chunk(), new Chunk(), new Chunk(), new Chunk()});

    final CustomersGroup customersGroup1 = new CustomersGroup();
    customersGroup1.setCustomers(new Customer[]{
                                     new Customer(), new Customer(), new Customer(),
                                     new Customer(), new Customer()});
    travel.setCustomersGroups(new CustomersGroup[]{customersGroup1});
    travel.setPrices(new Price[]{price, price});
    return travel;
}

Je sais d'après la méthode de test que si l'on passe ce Travel dans notre méthode compute(...), on doit obtenir 18.
En effet pour le tarif à exclure EXCLUDED_FARE, c'est la farePolicy2 qui comporte 3 Chunk et le CUSTOMER_TYPE_2 qui est concernée. Et la customerFare2 concerne 3 passagers de type CUSTOMER_TYPE_2.
Les deux Price étant identiques, on a

3 Chunk * 3 passagers + 3 Chunk * 3 passagers = 18

C'est en fait ce code de test qui m'a permis de comprendre l'implémentation (une preuve supplémentaire de pourquoi il faut tester et proprement de surcroît 😉 )
Même si le test est un peu complexe à comprendre il couvre une large palettes de cas.
On pourrait découper les cas de tests mais ce n'est pas l'objet de cet article.

Récapitulons

  • pour chaque Price
    • on liste les CustomerType concernés par les exclusions de tarif
    • pour chaque CustomerType à exclure
      • on cherche le FarePolicy correspondant à exclure
      • on cherche le CustomerFare correspondant à exclure
      • on combine le FarePolicy et le CustomerFare obtenus pour obtenir le nombre de Chunk à exclure
    • on additionne les nombres obtenus
  • on additionne le nombre obtenu pour chaque Price

Enfin un peu de Java 8

Convertissons tout ça en Java 8.
Premièrement, ce qui saute aux yeux : on doit avoir une fonction qui prend un Price qui retourne un int. Ce qui correspond à notre méthode countExclusionsForPrice précédente qu'on pourra référencer directement dans nos Stream via l'opérateur :: : this::countExclusionsForPrice
On verra son implémentation par la suite.

On aura une fonction qui combine le FarePolicy et le CustomerFare et qui retourne un int, une ToIntBiFunction<FarePolicy, CustomerFare>. On appliquera cette lambda à chaque couple FarePolicy, CustomerFare correspondant à une exclusion. Je ne sais pas encore quand et comment on pourra l'appliquer.

public int countExclusionsForFare(FarePolicy policy, CustomerFare faure) {
    return farePolicy.getChunks().length * customerFare.getCustomersQuantity();
}

Pour lister les CustomerType concernés par les exclusions de tarif pour un Price, là c'est facile : on liste l'ensemble des FarePolicy d'un Price, on ne garde que celles contenues dont le TarifLabel est contenu dans les tarifs exclus. Puis on récupère le CustomerType de chaque FarePolicy.
private Stream<CustomerType> findExcludedCustomerTypes(Price price) {
    return Stream.of(price.getFarePolicies())
            .filter(fp -> excludedFares().contains(fp.getFareName()))
            .map(FarePolicy::getCustomerType);
}
...
private List<FareLabel> excludedFares() {
    // retourne la liste des tarifs exclus
}

Essayons maintenant d'implémenter notre countExclusionsForPrice(Price) qui compte le nombre de Chunk à exclure pour un Price. Maintenant que j'ai récupéré les tarifs à exclure pour chaque Price, cherchons les combinaisons de FarePolicy et CustomerFare à partir de ces infos.
Celà signifie qu'à partir d'un Price et d'un CustomerType à exclure je dois :
1.retrouver le FarePolicy correspondant
2.retrouver le CustomerFare correspondant
3.enfin calculer le nombre de Chunk à exclure.

NB : En théorie, je devrais toujours avoir une correspondance 1-1 entre mes FarePolicy et mes CustomerFare et mes exclusions. N'en étant pas absolument certain, je vais utiliser des Optional<> pour accepter la correspondance 1-0. Il est par contre certain que je n'aurais pas de correspondance 1-n, je ne vais donc pas traiter le cas des listes.

Cela donne

public Optional<FarePolicy> findExcludedFarePolicy(final Price price, final CustomerType excluded) {
    return findExcludedItem(price, 
                            Price::getFarePolicies, 
                            c -> c.getCustomerType().equals(excluded));
}

public Optional<CustomerFare> findExcludedCustomerFare(final Price price, final CustomerType excluded) {
    return findExcludedItem(price, 
                            Price::getCustomerFares, 
                            c -> c.getType().equals(excluded));
}

private <I> Optional<I> findExcludedItem(Price price,
                                         Function<Price, I[]> itemsExtractor,
                                         Predicate<I> itemsFinder) {
    return Stream.of(itemsExtractor.apply(price))
            .filter(itemsFinder)
            .findFirst();
}

En ce qui concerne la countExclusionsForFare(FarePolicy,CustomerFare) et avec les Optional<> on aura ça

private int countExclusionsForFare(Optional<FarePolicy> farePolicy, Optional<CustomerFare> customerFare) {
    return farePolicy.isPresent() && customerFare.isPresent() ?
            farePolicy.get().getChunks().length 
                * customerFare.get().getCustomersQuantity() :
            DEFAULT;
}

En combinant tout ça, je peux calculer un nombre d'exclusions pour un Price et un CustomerType exclu
private int countExclusionsForPriceByCustomerType(Price price, CustomerType customerType) {
    final Optional<FarePolicy> farePolicy = findExcludedFarePolicy(price, customerType);
    final Optional<CustomerFare> customerFare = findExcludedCustomerFare(price,
                                                                         customerType);
    return this.countExclusionsForFare(farePolicy, customerFare);
}

En utilisant l'application partielle ou curryfication du Price, j'obtiens très simplement l'implémentation de ma fonctioncountExclusionsForPrice(Price) que je pourrais appliquer très simplement à un Stream des Price d'un Travel

private int countExclusionsForPrice(final Price price) {
    final Stream<CustomerType> excludedCustomerTypes = findExcludedCustomerTypes(price);
    return excludedCustomerTypes
            .mapToInt(
                 customerType -> this.countExclusionsForPriceByCustomerType(price,        
                                                                            customerType))
            .sum();
}

ce que l'on peut aussi écrire comme ça
private int countExclusionsForPrice(final Price price) {
    return Optional.of(price)
            .map(this::findExcludedCustomerTypes)
            .orElse(Stream.empty())
            .mapToInt(
                 customerType -> this.countExclusionsForPriceByCustomerType(price,
                                                                            customerType))
            .sum();
}

Ainsi pour obtenir le résultat, j'utilise le code suivant

public int count(final Travel travel) {
    if (!acceptPreconditions(travel)) {
        return DEFAULT;
    }
    return countChunksExclusions(travel.getPrices());
}

private int countChunksExclusions(Price... prices) {
    return Stream.of(prices)
            .mapToInt(this::countExclusionsForPrice)
            .sum();
}

Ou encore si on est joueur et en utilisant notre acceptPreconditions() juste pour tester le billing
private boolean acceptPreconditions() {
    return billing != null && !ArrayUtils.isEmpty(billing.getExcludedFares());
}

public int count(final Travel travel) {
    if(!acceptPreconditions()) {
        return DEFAULT;
    }
    return Optional.ofNullable(travel.getPrices())
            .map(Stream::of)
            .orElse(Stream.empty())
            .mapToInt(this::countExclusionsForPrice)
            .sum();
}

Conclusion

A première vue, le résultat n'est pas incroyable. En effet, il y a beaucoup de petites méthodes privées.

Les aspects négatifs

  • a première vue, c'est confus, il y a beaucoup de méthodes à peu de lignes
  • le code semble un peu explosé
  • il y a plus de lignes de codes

Le positif

  • au delà de l'aspect visuel (un simple méthode de 10 lignes, c'est plus beau que 10 méthodes de 1 ligne), la rentrée dans le code est plus simple
  • les concepts sont mieux définis et peuvent se composer, s'enchaîner (ce qui coûte en nombre de lignes de code)
  • le code est totalement composable, il est donc plus évolutif et de plus sans effet de bords

Ce que j’en retire

Malgré de nombreuses explications et lectures sur le sujet, grâce à cet exercice j'ai enfin compris/acquis en profondeur les concepts d'application partielle ou curryfication (c'est à dire que je me souviendrais ce que ça veut dire ;)) .
Autre chose, une fois les concepts à peu près maîtrisés, il est difficile de savoir quand s'arrêter d'écrire en fonctionnel. En effet, les notations Java 8 étant récentes, lire le code produit est parfois déroutant. Il faut donc savoir écrire du code à la hauteur de la maîtrise de l'équipe pour ne pas retrouver notre code à nouveau réécrit.
Par exemple, il n'est pas évident pour un néophyte de comprendre et penser qu'une méthode avec la signature suivante
int conversionEnInt(MonObjet) peut s'utiliser comme une ToIntFunction<MonObjet> lors de la manipulation d'un Stream<MonObjet>. J'en profite pour remercier Arnauld Loyer qui m'a fait remarquer que cet aspect des choses qui s'appliquait aux classes du JDK s'appliquait aussi aux miennes. Ce qui m'a permis de gagner en lisibilité.

À propos de clean code et de la lisibilité du Java 8, j'ai eu le plaisir d'assister à une présentation de Rémi Forax sur les Design Pattern reloaded en Java 8. Sa présentation bien que très intéressante finissait de manière très ardue. Lorsque je lui ai demandé si ce qu'il avait présenté était du clean code, il m'a répondu "c'est du clean code de demain".

J'ai beaucoup aimé cette réponse qui signifie pour moi que c'est normal si notre avis sur nos productions Java 8 évolue au cours du temps. Mais surtout, il faut essayer quitte à en faire un peu trop au début. En effet, c'est en poussant le bouchon et en ayant pas peur de déconstruire les raisonnements initiaux que j'ai fini par obtenir une version qui me semble raisonnable (même si elle peut être encore futuriste pour certains coéquipiers). Et oui j'ai présenté cet article comme si mon raisonnement avait été linéaire, ce qui est loin d'être le cas.

J'en profite pour remercier M. Cafetux pour sa relecture qui en m'ayant poussé dans mes retranchements m'a permis d'obtenir une version bien plus intéressante d'un point de vue fonctionnel.

Pour conclure, selon moi, grâce aux interfaces fonctionnelles, on a maintenant un code beaucoup plus expressif qu'avant. Au delà de quelques lourdeurs (ex : .stream()) liées à la rétrocompatibilité et ces nouvelles notations qu'il faut acquérir, je trouve que le code ainsi découpé est bien plus facilement lisible. On peut ensuite se plonger ou non dans les parties qui nous posent question.

Vous trouverez ci-dessous la version refactorée de ma classe

private Billing billing;
private final static int DEFAULT = 0;

public ChunksFromTravelBillingExcluder(final Billing billing) {
    this.billing = billing;
}

private boolean acceptPreconditions() {
    return billing != null && !ArrayUtils.isEmpty(billing.getExcludedFares());
}

public int count(final Travel travel) {
    if(!acceptPreconditions()) {
        return DEFAULT;
    }
    return Optional.ofNullable(travel.getPrices())
            .map(Stream::of)
            .orElse(Stream.empty())
            .mapToInt(this::countExclusionsForPrice)
            .sum();
}

private int countExclusionsForPrice(final Price price) {
    return Optional.of(price)
            .map(this::findExcludedCustomerTypes)
            .orElse(Stream.empty())
            .mapToInt(
                 customerType -> this.countExclusionsForPriceByCustomerType(price,
                                                                            customerType))
            .sum();
}

private int countExclusionsForPriceByCustomerType(Price price, CustomerType customerType) {
    final Optional<FarePolicy> farePolicy = findExcludedFarePolicy(price, customerType);
    final Optional<CustomerFare> customerFare = findExcludedCustomerFare(price, 
                                                                         customerType);
    return this.countExclusionsForFare(farePolicy, customerFare);
}

private int countExclusionsForFare(Optional<FarePolicy> farePolicy, Optional<CustomerFare> customerFare) {
    return farePolicy.isPresent() && customerFare.isPresent() ?
            farePolicy.get().getChunks().length 
                * customerFare.get().getCustomersQuantity() :
            DEFAULT;
}

private Optional<FarePolicy> findExcludedFarePolicy(final Price price, final CustomerType excluded) {
    return findExcludedItem(price, 
                            Price::getFarePolicies, 
                            c -> c.getCustomerType().equals(excluded));
}

private Optional<CustomerFare> findExcludedCustomerFare(final Price price, final CustomerType excluded) {
    return findExcludedItem(price, 
                            Price::getCustomerFares, 
                            c -> c.getType().equals(excluded));
}

private <I> Optional<I> findExcludedItem(Price price,
                                         Function<Price, I[]> itemsExtractor,
                                         Predicate<I> itemsFinder) {
    return Stream.of(itemsExtractor.apply(price))
            .filter(itemsFinder)
            .findFirst();
}

private Stream<CustomerType> findExcludedCustomerTypes(Price price) {
    return Stream.of(price.getFarePolicies())
            .filter(fp -> excludedFares().contains(fp.getFareName()))
            .map(FarePolicy::getCustomerType);
}

private List<FareLabel> excludedFares() {
    return Arrays.asList(billing.getExcludedFares());
}

Plus de publications

1 comment for “L’expressivité du fonctionnel avec Java 8

  1. Eldc
    27 octobre 2015 at 8 h 21 min

    // L’expressivité du fonctionnel avec C# 3.0 (2007) 😀
    int countChunksToExclude(Travel travel) {
    return (from price in travel.Prices
    from farePolicy in price.FarePolicies
    where billing.ExcludedFares.Contains(farePolicy.FareName)
    from customerFare in price.CustomerFares
    where customerFare.Type == farePolicy.PassengerType
    select farePolicy.Chunks.Length * customerFare.CustomersQuantity).Sum();
    }