mardi 14 juillet 2015

LMDP - Les petites phrases

Aujourd'hui, je vais m'attaquer à ces petites phrases, ces commentaires, qui peuvent prêter à sourire, mais qui masquent souvent un risque réel,. Et ça fait froid dans le dos. Exemple typique, la première de cette série...

Petit avertissement avant de commencer: tout ça s'est réellement passé

"Je me disais bien que ça ne fonctionnerait pas"

Le contexte est le suivant: un développeur avait terminé (c'est ce qu'il prétendait) le développement d'une application Web en Java et l'avait déployée dans l'environnement d'acceptance. Un collègue architecte effectue alors quelques tests et réussit à planter l'application (le beau plantage, avec affichage de l'exception sur l'écran de l'utilisateur).
Il appelle le développeur et lui montre: "là j'entre ça et je clique là et… (boum)". Le développeur contemple l'écran avec un petit sourire: "ah ça… Je me disais bien que ça ne fonctionnerait pas."

"Je n'en reçois que 170"

Attention, signe d'incompétence.

Cette phrase, c'est le cri d'alerte lancé par un responsable d'équipe de développement aux administrateurs DB. En temps qu'architecte, je suis en copie du mail (et de la réponse qui va suivre).

Ce responsable explique dans son mail qu'il interroge une base de données, mais en limitant le nombre de lignes renvoyées à 200. Il fournit aussi sa requête SQL. Son inquiétude vient du fait que le résultat n'est pas celui attendu: au lieu de recevoir les 200 lignes, il n'en reçoit que 170.

Réponse des DBA à l'intéressé: "ta table ne contient que 170 lignes".

"L'application sera multilingue grâce à…"

C'est l'histoire d'une application qui devait être personnalisée en fonction de la société qui l'utilisait: couleurs, logos et langues.

Au cours de nos premières réunions d'architecture, nous avons expliqué que le problème des chartes graphiques différentes serait résolu par l'utilisation de CSS différentes. Quant à la question du multilinguisme, elle était à la fois trop simple et trop complexe pour ces réunions préparatoires et fut laissée de côté.

Un directeur informatique suivait ces réunions d'architecture.

Au cours d'une présentation faite à l'ensemble du management, alors qu'on demandait aux architectes d'expliquer leurs premières constations, c'est ce directeur, sans doute pressé de se mettre en avant, qui prit la parole.

Il expliqua entre autres que "l'application serait multilingue grâce à l'utilisation de CSS adéquats". Les architectes, eux, regardaient leurs chaussures et se mordaient les joues pour rester sérieux.

"Même quand on connaît, c'est compliqué"

Alors qu'un chef de projet demandait à un responsable d'équipe de développement (le même qui voulait ses 200 lignes) de montrer le code d'une de ses applications à un développeur d'une autre équipe, il essuya un refus justifié en ces termes: "non, c'est trop compliqué, même quand on connaît, c'est compliqué".

Non mais… sérieusement quoi…

Pour terminer cette série de perles, voici trois petites phrases lues dans des rapports de progression, des documents très officiels sur l'état d'avancement du projet, envoyés au comité de pilotage et écrits par le chef de projet. Au fait, ce chef de projet était... le responsable d'équipe déjà cité.

"L'analyse fonctionnelle a débuté plus sérieusement la semaine du 4 mars". Et avant, c'était quoi? Une partie de rigolade?

Quatre mois plus tard, sur le même projet: "Un rapport de progression aurait dû être produit le 16 mais il est resté dans mes brouillons, mauvais point pour le chef de projet (c'était mon dernier jour avant mes vacances)".

Et finalement, 8 mois après le premier rapport, l'appréciation globale du projet est "moyenne". Pourquoi? C'est expliqué dans le rapport: "nous sommes loin d'être en avance". Et un peu plus loin, "cette semaine de test risque de s’étendre sur deux semaines".

Qui a dit que l'informatique n'était pas drôle?

jeudi 9 juillet 2015

LMDP: pas la bonne ligne

Ca faisait un bout de temps que j'avais envie de démarrer cette série. LMDP, c'est pour "Le meilleur du pire". Bon, je sais, comme nom de série, il y a sans doute mieux...

Depuis quelques années, je tiens une espèce de best of de pires bêtises que j'ai vue dans le milieu de l'informatique. Bien sûr, il y a du code, mais aussi de la gestion de projet (au sens large) et quelques inclassables.

C'est d'ailleurs par l'un d'eux que je vais commencer.

Le contexte

Tous les trois mois, nous migrons des clients d'une ancienne application vers une nouvelle. Tous les trois mois, un responsable de la migration nous demande de lui fournir des scripts SQL afin de reprendre les données de l'ancienne application vers la nouvelle.

Jusque-là, rien de très particulier.

Pour nous "aider", il extrait lui-même les données dans un fichier Excel. Chaque ligne du premier onglet représente une ensemble de données à insérer, parfois dans une, parfois dans deux tables.

Pour un des types de données, j'ai écrit une formule qui, copiée dans le deuxième onglet du document autant fois qu'il y a de lignes dans le premier, se transforme en requête SQL d'insertion. Je ne ferais pas ça tous les jours, mais bon...

Initialement, j'ajoutais la formule moi-même au fichier fourni par le responsable (appelons-le Antoine), mais comme ce n'est pas mon boulot, je lui ai dorénavant envoyé la formule afin qu'il bricole lui-même son fichier Excel. Et cela fonctionne très bien ainsi depuis plusieurs trimestres.

Première boulette

Quand je dis que tout se passe bien, ce n'est pas tout à fait vrai.

Le trimestre dernier, Antoine a demandé l'exécution de requêtes une première fois, puis s'est rendu compte que son fichier n'était pas complet, l'a complété, a généré les requêtes une nouvelle fois et a demandé une seconde exécution de ces requêtes.

Conséquence, des lignes dupliquées dans la base de données. Vous me direz que s'il y avait eu des contraintes d'unicité, ça ne serait pas arrivé, et vous auriez raison. A notre décharge, l'application vérifie que les lignes n'existent pas déjà et il paraissait impossible que ça arrive... jusque-là.

Nous décidons donc de revoir les requêtes de vérifier si la ligne n'existe pas déjà. J'effectue cette modification dans le dernier fichier fourni par Antoine.

Toujours dans l'urgence

Aujourd'hui, vers 10h, Antoine nous demande d'exécuter les scripts de migration. Petit vent de panique: la migration n'est normalement prévue qu'à la fin du mois et la moitié de l'équipe est d'ailleurs en congé, en particulier celui qui s'occupe d'une grande partie des migrations. Nous voudrions attendre mais rien à faire, les données doivent être migrées pour 12h, heure à laquelle les clients auront accès à la nouvelle application.

Néanmoins, on s'organise pour pallier le manque d'organisation d'Antoine...

Je lui rappelle que j'ai modifié la formule Excel. Il vient la voir sur mon PC, dans le fichier Excel de la dernière migration et me dit qu'il va m'envoyer le fichier, ce à quoi je réponds que je vais plutôt lui envoyer la formule. Avant qu'il ait eu le temps de réagir, j'ouvre un nouveau message et y copie la formule.

Là, il réagit et me dit: "envoie-moi aussi le fichier". Je lui réponds: "Tu dois plutôt utiliser la formule, c'est un ancien fichier". Il insiste et je la joins à l'e-mail.

Me voilà tranquille.

Deuxième boulette

Dix minutes plus tard, Antoine envoie un mail à l'équipe système pour exécuter un fichier de requêtes sql en production. Je suis en copie du mail.

Cinq minutes après, réponse de l'équipe système annonçant que les requêtes n'ont pas pu être exécutées.

Je jette un oeil au log d'erreur, puis aux requêtes envoyées par Antoine, et il apparaît que la dernière requête est incorrecte. Les paramètres qui devraient être insérés sont absents. J'en déduis que la formule a été copiée sur une ligne de trop, une ligne qui ne correspond à aucune donnée dans le premier onglet du fichier Excel.

Quelques minutes plus tard, nouveau mail d'Antoine, avec un fichier SQL, correct celui-là.

Bon, ce sont des choses qui arrivent...

Là où ça devient surréaliste, c'est que, après confirmation de l'exécution des requêtes en production, Antoine vient près de moi: "Il y avait un problème dans ton fichier. (Fichier qui est, je le rappelle, le sien, sur lequel j'ai ajouté la formule Excel). Tu commençais à la ligne 2 (Note: la première était un intitulé de colonne.). Le mien commençait à la ligne 1."

Ben tiens... CTRL+C, CTRL+V de la page de formule peut-être? Et tout ça directement en production, sans demander une simulation d'exécution en préproduction par exemple...

lundi 10 mars 2014

Modèle robuste: les relations dangereuses

Dans les articles précédents (JavaBean, la justification du mauvais orienté objet et Modèle robuste, immutabilité et Value Objects), j'ai montré comment créer un modèle robuste en laissant les objets assumer leurs responsabilités. Validation des paramètres, constructions atomiques, immutabilité ou value objects sont autant de patterns qui les aident à garantir leur cohérence.

Mais il est rare que les objets soient indépendants les uns des autres. Ils sont liés entre eux au travers de relations directes ou indirectes (via des collections), unidirectionnelles ou bidirectionnelles.

Un modèle est robuste si ses objets le sont, mais aussi si ses relations sont robustes. Dans l’ensemble, la gestion des relations n’est pas différente des autres propriétés. Après tout, sauf lorsqu’il s’agit de types primitifs, nous travaillons déjà avec des relations vers d’autres objets, même si nous nous sommes jusqu’à présent limités à des String ou d’autres Value Objects.

Relation simple


Le cas de base, c’est lorsqu’un objet A a une relation (une référence) vers un objet B. Dans ce cas, l’objet A doit veiller à la qualité de cette relation.

Par exemple, un objet Book a une relation vers un objet Category signifiant que le livre appartient à une catégorie. De plus, l'analyse du domaine montre que la catégorie DOIT être définie pour que l’objet livre soit cohérent.

Le code suivant est presque équivalent à celui de l’article précédent, sans nous encombrer du numéro Isbn. Pour simplifier cet exemple, le titre et l’auteur doivent être définis et immutables.
import org.apache.commons.lang3.StringUtils;

public class Book {
     private String title;
     private String author;
     private Category category;

     public Book(String title, String author, Category category) {
          if(StringUtils.isBlank(title) || StringUtils.isBlank(author)){
               throw new IllegalArgumentException("Ni le titre, ni l'auteur ne peut être null ou vide");
          }
          if(category == null){
               throw new IllegalArgumentException("La catégorie ne peut être null");
          }
          this.title = title.trim();
          this.author = author.trim();
          this.category = category;
     }

     public void setCategory(Category category) {
          if(category == null){
               throw new IllegalArgumentException("La catégorie ne peut être null");
          }
          this.category = category;
     }
     public Category getCategory() {
          return category;
     }
     public String getTitle() {
          return title;
     }
     public String getAuthor() {
          return author;
     }
}
Comme me l’a suggéré un collègue, je pourrais utiliser la méthode setCategory dans mon constructeur et éviter la duplication de la validation. Il faut toutefois être prudent: si j’héritais de la classe Book, je pourrais alors surcharger la méthode setCategory, la rendre par exemple moins restrictive, et construire des objets Book incohérents. Cela signifie que setCategory devrait alors être final.

Cette classe est robuste car elle garantit les conditions définies plus haut, y compris pour sa relation vers Category. Par contre, elle n’offre aucune garantie sur la qualité de la catégorie. Elle vérifie que null n’est pas passé en paramètre, mais pas que l’objet Category est cohérent.

C’est normal : chaque objet est responsable de sa propre cohérence. Ce n’est donc pas à Book de vérifier que l’objet Category est cohérent.
Cohérence et critères d'acceptation
Cela peut paraître un peu contradictoire, car pour le titre et l’auteur, nous vérifions que ces paramètres ne sont pas nuls et que les chaînes ne sont pas vides.

Cependant, nous ne vérifions pas la cohérence de la String : la String que nous recevons est cohérente de son point de vue. Une String contiendra toujours une chaîne de caractères, même s'il ne s'agit que d'espaces ("     ") ou de vide ("").

Il n’appartient pas à l’objet Book de vérifier la cohérence des String title ou author, mais il lui appartient de vérifier que leur contenu est acceptable pour lui.

Dans l’exemple ci-dessus, toute catégorie est acceptable puisque nous n’y avons mis aucune contrainte. Mais imaginons que nous ayons une classe ChildrenBook, un livre pour enfant. Nous ne voulons pas pouvoir lui attribuer une catégorie "gore" ou "érotique".

Pour remplir ce besoin, nous allons créer une propriété booléenne "adult" dans Category, à laquelle nous accèderons via un "isAdult". Quant à notre ChildrenBook, il étend désormais Book:
public class ChildrenBook extends Book{
     public ChildrenBook(String title, String author, Category category) {
          super(title, author, category);
          if(category.isAdult()){
               throw new IllegalArgumentException("pas de catégorie pour adulte dans un livre pour enfant");
          }
     }

     public final void setCategory(Category category) {
          if(category.isAdult()){
               throw new IllegalArgumentException("pas de catégorie pour adulte dans un livre pour enfant");
          }
          super.setCategory(category);
     }
}
A nouveau, ChildrenBook ne vérifie pas la cohérence de Category, mais vérifie que la catégorie passée en paramètre est acceptable pour lui.

Soit dit en passant, heureusement que setCategory n’était pas final… Cela nous permet de valider que notre catégorie est acceptable.

Relation multiple


Dans le cas d’une relation multiple, les choses sont un peu plus complexes. L’objet A référencie une collection d’objets B. Et le problème, c’est justement cette collection.

Etendons notre exemple de base: un livre appartient désormais à plusieurs catégories. Comme ci-dessus, nous nous contenterons de vérifier qu’aucune de ces catégories n’est nulle. On pourrait y ajouter un contrôle sur le contenu de catégorie, comme pour les livres pour enfants, le problème serait le même.
public class Book {
     private String title;
     private String author;
     private List<Category> categories;

     public Book(String title, String author, List<Category> categories) {
          if(StringUtils.isBlank(title) || StringUtils.isBlank(author)){
               throw new IllegalArgumentException("Ni le titre, ni l'auteur ne peut être null ou vide");
          }
          if(categories ==null || categories.isEmpty()){
               throw new IllegalArgumentException("La liste de catégories ne peut être nulle ou vide");
          }
          for(Category c:categories){
               if(c == null){
                    throw new IllegalArgumentException("Aucune élément de la liste de catégories ne peut être null");
               }
          }
          this.title = title.trim();
          this.author = author.trim();
          this.categories = categories;
     }

     public List<Category> getCategories() {
          return categories;
     }
     public String getTitle() {
          return title;
     }
     public String getAuthor() {
          return author;
     }
}
Cette implémentation semble respecter le contrat que nous nous étions fixé. Dans le cas présent, il n’y a pas de setter sur categories, ce qui fait qu’une fois la liste assignée, on ne peut plus en changer.

Attention, cela ne signifie pas que je ne peux pas ajouter de catégories ! Pire, cela ne signifie pas que je ne peux pas ajouter de catégorie null !

C’est même assez simple :
book.getCaterories().add(nouvelleCategorie) ; //Ajoute une catégorie aux catégories de book
book.getCategories().add(null); // Exactement ce qu’on ne voulait pas !
Le problème, c’est que le getCategories expose la collection de catégories, laquelle ne veille pas sur la qualité des éléments qui lui sont ajoutés. Normal, ce n’est pas son rôle.

Une solution consisterait à créer sa propre collection, en implémentant List ou en étendant ArrayList et en surchargeant la méthode add pour qu’elle vérifie les null.

Une autre possibilité, plus simple à mon avis, est de modifier l’implémentation de la méthode getCategories et, si le domaine permet l’ajout de catégories à un livre existant, d’implémenter la méthode addCategory de la manière suivante :
public void addCategory(Category category){
     if(category == null){
          throw new IllegalArgumentException("Une catégorie ne peut être nulle");
     }
     categories.add(category);
}

public List<Category> getCategories() {
     return Collections.unmodifiableList(categories);
}
La méthode addCategory vérifie la qualité des catégories ajoutées. Aucun null ne sera permis.

Quant à getCategories, elle renvoie maintenant une collection immutable contenant bien les catégories du livre. Plus question de l'utiliser pour modifier la collection, le développeur recevra une exception.

Parfait ? Non, il reste un problème important, illustré par le test (TestNg) suivant :
@Test
public void testAddNullCategoryAnyway(){
     Category sf = new Category("sf","Science-fiction");
     List<Category> cats = new ArrayList<Category>();
     cats.add(sf);

     Book b = new Book("Dune","Frank Herbert",cats);

     assertEquals(b.getCategories().size(), 1);

     cats.add(null); //Aie !

     assertEquals(b.getCategories().size(), 2, "Hé oui...");

     assertTrue(b.getCategories().contains(null), "Pire...");
}
Bien sûr, rappelez-vous, dans le constructeur :
this.categories = categories;
Donc, toute modification de la collection originale modifiera la liste des catégories du livre, sans vérifier la qualité de la modification.

La solution est simple, il suffit de remplacer la ligne précédente par celle-ci :
this.categories = new ArrayList<Category>(categories);
Hé oui ! Travailler avec des collections n’est pas simple.

Mais il y a pire : les relations bidirectionnelles.

Relation bidirectionnelle


Si un livre appartient à une ou plusieurs catégories et que chaque catégorie contient un ou plusieurs livres, la relation est dite bidirectionnelle. A noter que cela n’a rien à voir avec la cardinalité, juste avec le fait qu’un objet A possède une ou plusieurs références vers des objets B qui possèdent une ou plusieurs références vers l’objet A.

Le piège de ce type de relation, c’est qu’elle paraît toujours logique : un livre appartient à des catégories, et donc ces catégories contiennent le livre ; un chien a un maître, donc le maître a un chien; une facture a plusieurs lignes, donc chaque ligne appartient à une facture…

Les développeurs ont donc tendance à l’implémenter partout.

Mais logique ne veut pas dire intéressante. La question à se poser avant d’implémenter une relation bidirectionnelle, c’est de savoir si les deux navigations qu’elle propose (de A vers B ou de B vers A) sont aussi intéressantes l’une que l’autre. C’est l’analyse du domaine applicatif qui répondra à cette question.

Les relations bidirectionnelles sont surtout délicates à gérer. Outre les aspects déjà cités, il faut aussi tenir compte de la cohérence de la bidirectionnalité: si A a une référence vers B, alors B DOIT avoir une référence vers A.

Cela peut sembler superflu, mais je vois souvent des problèmes avec Hibernate qui sont simplement dus à une incohérence dans la relation bidirectionnelle.

Laissons Hibernate de côté et voyons comment gérer de manière robuste nos relations bidirectionnelles.

Gardons les principes de robustesse précédents et voyons le code pour Book et Category.
public class Category {
     private String name;
     private String description;
     private Set<Book> books;

     public Category(String name, String description) {
          if(StringUtils.isBlank(name)){
               throw new IllegalArgumentException("Le nom ne peut null ou vide");
          }
          this.name = name.trim();
          this.description = StringUtils.trimToNull(description);
          this.books = new HashSet<Book>();
     }

     public String getName() {
          return name;
     }
     public String getDescription() {
          return description;
     }
     public Set<Book> getBooks() {
          return Collections.unmodifiableSet(books);
     }

     public void addBook(Book book){
          if(book == null){
               throw new IllegalArgumentException("Le livre ne peut être null");
          }

          this.books.add(book);
          book.addCategory(this);
     }

     public int hashCode() {
          return name.hashCode();
     }

     public boolean equals(Object obj) {
          if (this == obj)
               return true;
          if (obj == null)
               return false;
          if (!(obj instanceof Category))
               return false;
          return name.equals(((Category) obj).name);
     }
}
public class Book {
     private String title;
     private String author;
     private Set<Category> categories;

     public Book(String title, String author) {
          if(StringUtils.isBlank(title) || StringUtils.isBlank(author)){
               throw new IllegalArgumentException("Ni le titre, ni l'auteur ne peut être null ou vide");
          }
          this.title = title.trim();
          this.author = author.trim();
          this.categories = new HashSet<Category>();
     }

     void addCategory(Category category){
          if(!category.getBooks().contains(this)){
               throw new IllegalArgumentException("La catégorie doit contenir le livre. Passez plutôt par category.addBook.");
          }
          categories.add(category);
     }

     public Set<Category> getCategories() {
          return Collections.unmodifiableSet(categories);
     }

     public String getTitle() {
          return title;
     }
     public String getAuthor() {
          return author;
     }

     public int hashCode() {
          return title.hashCode();
     }

     public boolean equals(Object obj) {
          if (this == obj)
               return true;
          if (obj == null)
               return false;
          if (!(obj instanceof Book))
               return false;

          return title.equals(((Book) obj).title);
     }
}
Que remarque-t-on ?
  • La collection de catégories n’est plus une List, mais un Set (idem pour les livres dans les catégories). Ce n’est pas absolument nécessaire et ça aurait pu être fait auparavant. Ca permet d’éviter facilement les doublons (ajouter deux fois le même livre à une catégorie) à condition d’implémenter correctement les méthodes equals/hashcode. Dans le cas présent, Book n’a plus vraiment d’attribut adéquat (Isbn en était un) et le equals a été fait sur le titre, ce qui n’est pas correct dans le monde réel. Au niveau Category, on peut supposer que le nom d’une catégorie est unique. Une petite remarque au passage : je trouve que les développeurs utilisent plus facilement des List que des Set, et c’est dommage. Car imaginez ce code avec des List : lourd…
  • Le code part du principe qu’un livre est ajouté à une catégorie et non l’inverse (on aurait facilement pu travailler dans l’autre sens). Il n’est donc normalement pas possible (c’est-à-dire pas des conditions normales) d’ajouter une catégorie à un livre.
  • La méthode addCategory existe pourtant bien dans Book, mais sa visibilité est réduite au package. C’est un pis-aller (la visibilité devrait être réduite à la classe Category, ce que Java ne permet pas de faire). Elle est destinée à n'être appelée que par Category pour garantir la bidirectionnalité. Dans le cas où elle serait malgré tout appelée (depuis une autre classe appartenant au même package), elle vérifie que la Category contient bien le livre, sans quoi elle lance une exception conseillant de passer par addBook de Category. Elle ne vérifie plus que le paramètre n’est pas null (puisqu’il ne peut plus l’être).
  • La méthode addBook de Category ajoute le livre et appelle addCategory de Book pour garantir la bidirectionnalité.
  • Il est encore plus important que jamais d’interdire l’ajout direct d’un objet dans les collections books et categories en passant par getBooks ou getCategories() !
A part cette visibilité "package", ce code est plutôt robuste et assez simple à développer. Il existe d’autres options qui éliminent le risque précécédent.
Permettre le addBook et le addCategory
Plutôt que de prendre le risque d’appeler de manière illégale la méthode addCategory de Book, bien que cet appel garantisse lui-même la cohérence de la relation, une autre approche consiste à autoriser aussi bien addBook que addCategory.

La méthode addBokk devient :
public void addBook(Book book){
     if(book == null){
          throw new IllegalArgumentException("Le livre ne peut être null");
     }

     this.books.add(book);
     if(! book.getCategories().contains(this)){
          book.addCategory(this);
     }
}
La method addCategory, maintenant publique, devient:
public void addCategory(Category category){
     if(category == null){
          throw new IllegalArgumentException("La catégorie ne peut être nulle");
     }
     categories.add(category);
     if(! category.getBooks().contains(this)){
          category.addBook(this);
     }
}
Afin d’éviter une boucle sans fin, il faut vérifier si l’ajout a déjà été fait de l’autre côté et, si ce n'est pas le cas, établir la bidirectionnalité.
Réflexion
Une dernière approche consiste à éliminer le risque d’appel à addCategory, en mettant la méthode en private. La méthode addBook de Category devient alors :
public void addBook(Book book){
     if(book == null){
          throw new IllegalArgumentException("Le livre ne peut être null");
     }

     try {
          Method addCategoryMethod = Book.class.getDeclaredMethod("addCategory", Category.class);
          addCategoryMethod.setAccessible(true);
          addCategoryMethod.invoke(book, this);
          this.books.add(book);
     } catch (SecurityException e) {
          throw new RuntimeException("TODO...",e);
     } catch (NoSuchMethodException e) {
          throw new RuntimeException("TODO...",e);
     } catch (IllegalArgumentException e) {
          throw new RuntimeException("TODO...",e);
     } catch (IllegalAccessException e) {
          throw new RuntimeException("TODO...",e);
     } catch (InvocationTargetException e) {
          throw new RuntimeException("TODO...",e);
     }
}
Je pense que c’est un des rares cas où la réflexion est tout à fait acceptable dans le cadre du développement du modèle.

Certaines optimisations sont possibles, comme charger l’objet addCategoryMethod dans le constructeur de la classe Category, voire dans une initialisation statique.

Certains pourraient être tentés de ne pas implémenter de méthode addCategory dans Book et de modifier directement, par réflexion, la collection de Category de Book.

C’est bien sûr possible, mais je le déconseille. Même si l’on triche un peu en utilisant la réflexion, on laisse le soin au livre de veiller à la cohérence de ses attributs.

Et le modèle est robuste.

dimanche 23 février 2014

Et un Mockito ! Un !

Cet article est une version mise à jour d'un article initialement paru sur le Coin de l'architecte Java.

Quand j'écris un test, je veux qu'il soit isolé autant que possible, à savoir qu'il teste une méthode sans que les résultats des méthodes d'autres objets dont elle dépend et leurs erreurs éventuelles ne viennent perturber ses résultats. Celles-là feront l'objet d'autres tests.

Pour cela j'utilise des mocks pour remplacer les dépendances. Je ne cherche pas à lancer une discussion sur le bienfondé de cette manière de travailler, ni sur ce qu'il convient ou non de mocker. Non, mon but ici est de présenter Mockito, la librairie que j'utilise pour cela.

Pourquoi ai-je préféré Mockito à une autre librairie? Pour plusieurs raisons. Sans prétendre avoir fait une recherche exhaustive sur le sujet, je trouve que Mockito est assez légère (par rapport à EasyMock par exemple) et puis j'aime beaucoup sa manière de configurer des mocks avec un DSL Java.

Pour exécuter les exemples qui suivent, je vous conseille de créer un projet Maven et d'ajouter dans ses dépendances:
<dependency>
     <groupId>org.mockito</groupId>
     <artifactId>mockito-all</artifactId>
     <version>1.9.5</version>
     <scope>test</scope>
</dependency>
Comme il s'agit d'une dépendance utilisée pour des tests unitaires, le scope sera bien entendu "test". A noter que j'utilise aussi TestNg (ce qui n'est pas nécessaire pour Mockito... mais j'ai mes habitudes).

Les tests porteront sur une classe toute simple, une interface en fait, que je crée dans un package quelconque de src/main/java. Il ne faut pas y chercher d'autre logique que de pouvoir tester le fonctionnement de Mockito.
public interface SimpleClass {
     Integer methodWithInteger(String message);
     int methodWithInt(String message);
     boolean methodWithBoolean(String message);
     Boolean methodWithBooleanWrapper(String message);
     String methodWithString(String message);
     Object methodWithObject(String message);
}

Création d'un mock


Pour tester le fonctionnement de Mockito, je vais écrire des tests unitaires (TestNg), qui seront évidemment dans src/test/java. Voici le premier:
import static org.mockito.Mockito.*;
import static org.testng.Assert.*;

public class TestMock {
     @Test
     public void testNotImplementedResults(){
          SimpleClass mock = mock(SimpleClass.class);

          assertEquals(mock.methodWithInteger("hello"), Integer.valueOf(0));
          assertEquals(mock.methodWithInt("hello"),0);
          assertFalse(mock.methodWithBoolean("hello"));
          assertEquals(mock.methodWithBooleanWrapper("hello"),Boolean.FALSE);
          assertNull(mock.methodWithString("hello"));
          assertNull(mock.methodWithObject("hello"));
     }
}
Les imports statiques permettent d'utiliser très simplement les méthodes statiques de la classe Mockito (le reste est là pour testng.Assert). Dans le cas présent, nous utilisons la méthode "mock" qui crée un "mock" de notre interface/classe de base.

L'exécution de ce test (qui réussit), nous montre plusieurs choses:
  • les méthodes existent (avec d'autres frameworks, l'appel d'une méthode sans l'avoir définie au niveau du mock aurait provoqué une exception. Il y a du pour et du contre.)
  • il n'y a aucune implémentation de l'interface, c'est Mockito qui fournit l'implémentation. Si SimpleClass était une classe concrète, avec des implémentations de ses méthodes, aucune ne serait de toute façon appelée. Le mock de Mockito intercepte tous les appels.
  • la valeur renvoyée dans tous les cas est nulle ou équivalente. Pour un type primitif ou son wrapper, c'est 0 (false pour un booléen). Si c'est un objet (autre qu'un Wrapper), c'est null qui est renvoyé.
Vous me direz: "voilà un mock qui ne fait pas grand-chose". Vraiment? En fait, il en fait déjà pas mal.

Vérification des appels


Ce simple mock permet en fait de vérifier qu'une méthode est appelée ou non. Le test suivant le montre:
@Test
public void testVerifyCalledMethod(){
     SimpleClass mock = mock(SimpleClass.class);

     mock.methodWithInteger("hello");
     verify(mock).methodWithInteger("hello");
}
Cela signifie donc que la méthode "methodWithInteger" a bien été appelée.

Par contre, le test suivant échoue:
@Test
public void testVerifyNotCalledMethod(){
     SimpleClass mock = mock(SimpleClass.class);

     mock.methodWithInteger("hello");
     verify(mock).methodWithString("hello");
}
avec la sortie console suivante (les numéro de ligne peuvent être différents):
FAILED: testVerifyNotCalledMethod
Wanted but not invoked:
simpleClass.methodWithString("hello");
-> at mockito.TestMock.testVerifyNotCalledMethod(TestMock.java:37)
La méthode verify vérifie donc bien que la méthode "surveillée" a été appelée, mais il y a mieux.

Le test suivant échoue:
@Test
public void testVerifyCalledMethodButWithDifferentArgument(){
     SimpleClass mock = mock(SimpleClass.class);

     mock.methodWithInteger("hello");
     verify(mock).methodWithInteger("toto");
}
avec la sortie console:
FAILED: testVerifyCalledMethodButWithDifferentArgument
Argument(s) are different! Wanted:
simpleClass.methodWithInteger("toto");
-> at mockito.TestMock.testVerifyCalledMethodButWithDifferentArgument(TestMock.java:45)
Actual invocation has different arguments:
simpleClass.methodWithInteger("hello");
Donc la méthode verifiy vérifie que la méthode du mock a été appelée, mais aussi avec quels arguments. Nous verrons dans un instant qu'elle en fait fait un peu plus.

Cette fonctionnalité est utile pour vérifier qu'une méthode d'une dépendance de l'objet testé, dépendance remplacée par un mock, a bien été appelée, avec les bons paramètres.

En attendant, nous avons éventuellement un problème.

Vérification par type de paramètre


Si nous souhaitons vérifier qu'une méthode est appelée avec un paramètre bien défini, comme ci-dessus, c'est bien. Cependant, la String exacte passée en paramètre n'est parfois pas connue avant le test ou sans importance. Comment vérifier que la méthode a été appelée avec une String, quelle qu'elle soit?

Comme ceci:
@Test
public void testVerifyCalledMethodWithAnyParameterValue(){
     SimpleClass mock = mock(SimpleClass.class);

     mock.methodWithInteger("hello");
     verify(mock).methodWithInteger(anyString());
}
"anyString()" est un "matcher" (et d'une certaine manière, on pourrait dire qu'un paramètre tout simple - une String dans notre cas - est une forme de matcher).

Il en existe d'autres semblables à anyString(): anyInt(), anyFloat(), anyObject(), any()... Voyez les méthodes statiques des classes org.mockito.Matchers et org.mockito.AdditionalMatchers pour un aperçu de toutes les possibilités.

Le code suivant est équivalent au précédent:
verify(mock).methodWithInteger(any(String.class));
Un autre exemple de matcher:
@Test
public void testVerifyCalledMethodWithStringValueBeginningWith(){
     SimpleClass mock = mock(SimpleClass.class);

     mock.methodWithInteger("bonsoir");
     verify(mock).methodWithInteger(startsWith("bon"));
}
fonctionne pour "bonjour", "bonsoir" mais pas "au revoir"...

Et on peut en plus créer ses propres matchers en étendant la classe org.hamcrest.BaseMatcher:
public class SalutationMatcher extends BaseMatcher<String>{
     public boolean matches(Object item) {
          boolean match = false;
          if(item instanceof String){
               String s = (String)item;
               match = s.startsWith("b") && s.endsWith("r");
          }
          return match;
     }

     public void describeTo(Description description) {
          description.appendText("String qui commence par 'b' et se termine par 'r'");
     }
}
dans laquelle:
  • "matches" effectue la vérification (ici, la String doit commencer par un "b" et se terminer par un "r").
  • "describeTo" permet de décrire le matcher, c'est-à-dire le texte qui sera affiché si le matcher n'est pas "matché".
Voici deux tests qui utilisent ce matcher:
@Test
public void testVerifyCallWithSalutation(){
     SimpleClass mock = mock(SimpleClass.class);

     mock.methodWithInteger("bonsoir");
     verify(mock).methodWithInteger(argThat(new SalutationMatcher()));
}

@Test
public void testVerifyWithNotASalutation(){
     SimpleClass mock = mock(SimpleClass.class);

     mock.methodWithInteger("bon appétit");
     verify(mock).methodWithInteger(argThat(new SalutationMatcher()));
}
Le premier réussit. Il aurait aussi réussi avec "bonjour".

Le deuxième échoue, avec cette indication dans la console:
FAILED: testVerifyWithNotASalutation
Argument(s) are different! Wanted:
simpleClass.methodWithInteger(
    String qui commence par 'b' et se termine par 'r'
);
-> at mockito.TestMock.testVerifyWithNotASalutation(TestMock.java:78)
Actual invocation has different arguments:
simpleClass.methodWithInteger(
    "bon appétit"
);
Ce qui était "wanted" correspond au texte qu'on a écrit dans la description.

Vérification du nombre d'appels


Peut-être avez-vous déjà essayé quelque chose comme ceci:
@Test
public void testSomethingGoesWrongWhenVerifyingCallsOnMethodCallTwice(){
     SimpleClass mock = mock(SimpleClass.class);

     mock.methodWithInteger("hello");
     mock.methodWithInteger("bonjour");
     verify(mock).methodWithInteger(anyString());
}
Ce test echoue:
FAILED: testSomethingGoesWrongWhenVerifyingCallsOnMethodCallTwice
org.mockito.exceptions.verification.TooManyActualInvocations: 
simpleClass.methodWithInteger();
Wanted 1 time:
-> at mockito.TestMock.testSomethingGoesWrongWhenVerifyingCallsOnMethodCallTwice(TestMock.java:87)
But was 2 times. Undesired invocation:
-> at mockito.TestMock.testSomethingGoesWrongWhenVerifyingCallsOnMethodCallTwice(TestMock.java:86)
C'est parce que la méthode "verify" vérifie que la méthode a bien été appelée, mais aussi qu'elle a été appelée une et une seule fois.

Il peut en effet être intéressant de vérifier qu'une méthode a été appelée un certain nombre de fois.
@Test
public void testVerifyNumberOfInvocations(){
     SimpleClass mock = mock(SimpleClass.class);

     mock.methodWithInteger("bonjour");
     mock.methodWithInteger("bonsoir");

     verify(mock,times(2)).methodWithInteger(anyString());
     verify(mock,atLeastOnce()).methodWithInteger(anyString());
     verify(mock,times(1)).methodWithInteger("bonjour");
     verify(mock,atLeastOnce()).methodWithInteger("bonjour");
     verify(mock).methodWithInteger("bonsoir");
     verify(mock,never()).methodWithInteger("au revoir");
}
Tout ce qui précède fonctionne:
  • le premier verify vérifie que la méthode a été appelée exactement deux fois avec n'importe quelle String;
  • le deuxième que la méthode a été appelée au moins une fois avec n'importe quelle String;
  • le troisième que la méthode a été appelée une et une seule fois avec "bonjour";
  • le quatrième que la méthode a été appelée au moins une fois avec "bonjour";
  • le cinquième fait comme le troisième, mais avec "bonsoir", autrement dit "times(1)" est optionnel;
  • le sixième vérifie que la méthode n'a jamais été appelée avec "au revoir" comme paramètre (ce qui équivaut à "times(0)")
Il existe d'autres possibilités: voir les méthodes statiques de la classe org.mockito.Mockito.

Les stubs


Jusqu'à présent, nous avons vérifié l'appel des méthodes et nous ne nous sommes pas intéressés à ce qu'elles renvoyaient, d'autant plus que, jusqu'à présent elles ne renvoyaient que null ou ses équivalents.

Mais un grand intérêt des mocks dans des tests est de leur faire renvoyer des valeurs que nos "Classes Under Test" utiliseront.

Pour cela, il faut définir des "stubs" pour ces méthodes.

Voici un test qui montre le fonctionnement:
@Test
public void testSimpleStubbing(){
     //Création d'un mock
     SimpleClass mock = mock(SimpleClass.class);
     //Définition des stubs
     when(mock.methodWithInteger("bonjour")).thenReturn(Integer.valueOf(1));
     when(mock.methodWithInteger("bonsoir")).thenReturn(Integer.valueOf(2));

     //Test
     assertEquals(mock.methodWithInteger("bonjour"),Integer.valueOf(1));
     assertEquals(mock.methodWithInteger("bonsoir"),Integer.valueOf(2));
     assertEquals(mock.methodWithInteger("hello"),Integer.valueOf(0));
}
La paramètre du stub est évidemment un "matcher" et on peut donc écrire:
when(mock.methodWithInt(anyString()).thenReturn(3);
On peut aussi réutiliser le matcher que nous avons écrit plus haut:
when(mock.methodWithString(argThat(new SalutationMatcher()))).thenReturn("hello");
Cela ne vous satisfait pas? Vous voulez une réponse calculée depuis la valeur du paramètre? Nous verrons plus loin l'implémentation partielle d'une méthode . Dans l'immédiat, la solution consiste à utiliser une "Answer".
@Test
public void testStubbingWithAnswer(){
     SimpleClass mock = mock(SimpleClass.class);
     when(mock.methodWithInteger(anyString())).thenAnswer(new Answer<integer>(){
          public Integer answer(InvocationOnMock invocation) throws Throwable {
               String param = (String) invocation.getArguments()[0];
               if(param.startsWith("bon"))
                    return Integer.valueOf(2);
               else
                    return Integer.valueOf(1);
               }}
          );

     assertEquals(mock.methodWithInteger("bonjour"),Integer.valueOf(2));
     assertEquals(mock.methodWithInteger("bonsoir"),Integer.valueOf(2));
     assertEquals(mock.methodWithInteger("hello"),Integer.valueOf(1));
}
Il est aussi possible de donner une liste de réponses qui seront renvoyées à chaque appel:
@Test
public void testStubbingWithListOfAnswers(){
     SimpleClass mock = mock(SimpleClass.class);
     when(mock.methodWithInt(anyString())).thenReturn(1,2,3);

     assertEquals(mock.methodWithInt("hello"),1);
     assertEquals(mock.methodWithInt("hello"),2);
     assertEquals(mock.methodWithInt("hello"),3);
     assertEquals(mock.methodWithInt("hello"),3);
}
S'il y a plus d'appels que la liste ne contient de valeurs, la dernière sera renvoyée pour tous les appels subséquents.

Parfois, un mock devra lancer une exception. Par exemple:
@Test
public void testStubbingWithException(){
     SimpleClass mock = mock(SimpleClass.class);
     when(mock.methodWithInt(anyString())).thenReturn(1);
     when(mock.methodWithInt("bye bye")).thenThrow(new RuntimeException("pas bye bye"));

     assertEquals(mock.methodWithInt("hello"),1);

     try{
          mock.methodWithInt("bye bye");
          fail("Une exception doit avoir été lancée");
     }catch(RuntimeException e){
          assertEquals(e.getMessage(),"pas bye bye");
     }
}
L'exception définie doit être compatible avec la signature de la méthode stubbée. Ici, comme la signature de methodWithInt ne déclare aucune exception (throws Exception, par exemple), je ne peux utiliser qu'une exception de type RuntimeException.

L'exemple ci-dessus montre aussi qu'il est possible de surcharger le comportement d'un stub. Dans un premier temps, un résultat pour anyString est défini. Ensuite, un résultat spécifique à "bye bye" est défini.

Implémenter partiellement une méthode


Ajoutons une classe basique:
public class Personne {
     private String nom;

     public Personne(String nom){
          this.nom = nom;
     }

     public String getNom() {
          return nom;
     }

     public void setNom(String nom) {
          this.nom = nom;
     }
}
et une méthode dans SimpleClass:
int compteEtCapitalise(Personne p);
Son contrat spécifie qu'elle capitalise le nom d'une personne et renvoie le nombre de lettres dans son nom.

Bon, ça n'a aucun intérêt dans le monde réel. Imaginons cependant que notre classe testée dépende de SimpleClass et utilise cette méthode compteEtCapitalise. L'effet "de bord" (la mise en capitales du nom), elle s'en moque, mais elle dépend du résultat renvoyé, lequel doit être cohérent par rapport à l'objet Personne envoyé.

Or, ce n'est pas facilement le cas.

Cette version n'est pas satisfaisante:
@Test
public void testCheckAnswerCoherence(){
     SimpleClass mock = mock(SimpleClass.class);
     when(mock.compteEtCapitalise(any(Personne.class))).thenReturn(4);

     assertEquals(mock.compteEtCapitalise(new Personne("toto")),4);
     assertEquals(mock.compteEtCapitalise(new Personne("marcel")),4, "Et non 6...");
}
La valeur renvoyée est fixée à 4 et ne sera généralement pas correcte. Soyons clair, il y a moyen de contourner ce problème, au prix d'une configuration plus lourde: une pour "toto" et une autre pour "marcel". Mais si le nom de la Personne passée en paramètre est imprévisible (même si pour moi, c'est un problème de qualité du test, mais c'est une autre histoire), ça ne fonctionne pas.

doAnswer


Ce dont nous avons besoin, c'est d'implémenter partiellement la méthode.
@Test
public void testCheckAnswerCoherence(){
     SimpleClass mock = mock(SimpleClass.class);
     when(mock.compteEtCapitalise(any(Personne.class))).thenAnswer(new Answer(){
          public Integer answer(InvocationOnMock invocation) throws Throwable {
               Personne p = (Personne)invocation.getArguments()[0];
               //p.setNom(p.getNom().toUpperCase()); //A décommenter si l'effet de bord est important
               return p.getNom().length();
          }
     });

     assertEquals(mock.compteEtCapitalise(new Personne("toto")),4);
     assertEquals(mock.compteEtCapitalise(new Personne("marcel")),6);
}
On peut se poser la question de l'utilité du mock si dans les faits on doit implémenter (même partiellement) une ou plusieurs de ses méthodes. Ce cas de figure est à réserver aux cas où la méthode "stubbée" et implémentée sera beaucoup plus basique que la méthode réelle et qu'il est absolument nécessaire que certaines opérations qu'elle ferait en réalité soient effectuées.

Néanmoins, il est peut-être plus logique de créer une implémentation "mock" de l'interface sans passer par Mockito. C'est à voir...

My name is Bond


Si vous avez bien suivi, le test suivant ne vous pose aucun problème:
@Test
public void testAnotherAnswerIncoherence(){
     List<String> mock = mock(List.class);
     mock.add("hello");
     mock.add("bonjour");

     verify(mock,times(2)).add(anyString());
     assertEquals(mock.size(),0);
}
Malgré l'ajout de deux String à mon mock, l'appel de size renvoie 0. C'est normal: sans autre indication, un stub renvoie toujours une valeur nulle.

N'imaginez même pas utiliser la méthode doAnswer pour renvoyer un résultat cohérent, mais un espion peut vous aider.
@Test
public void testSpyBehaviour(){
     List<String> liste = new ArrayList<String>();
     List<String> spy = spy(liste);

     spy.add("hello");
     spy.add("bonjour");

     verify(spy,times(2)).add(anyString());
     assertEquals(spy.size(),2);
}
Ces deux exemples montre le fonctionnement différent des mocks et des spies. En pratique, on utilisera un spy lorsque l'instance existe déjà, et que l'on souhaite vérifier (verify) si elle a été appelée, mais aussi éventuellement modifier son comportement (stub).

Car l'espion n'est pas un mock mais il est possible de "stubber" ses méthodes:
@Test
public void testSpyAndStub(){
     List<String> liste = new ArrayList<String>();
     List<String> spy = spy(liste);
     when(spy.size()).thenReturn(100);

     spy.add("hello");
     spy.add("bonjour");

     verify(spy,times(2)).add(anyString());
     assertEquals(spy.size(),100);
}
Cette fois, le résultat affiché est 100. La méthode size réelle a été remplacée par un stub qui renvoie toujours 100. Intérêt? Aucun dans le cas présent...

Attention


Essayons ceci:
@Test
public void testIllegalStubbingOnSpy(){
     List<String> liste = new ArrayList<String>();
     List<String> spy = spy(liste);
     when(spy.get(0)).thenReturn("hello");
     assertEquals(spy.get(0),"hello");
}
Ce test échoue en provoquant un IndexOutOfBoundsException sur le ligne du "when", car la méthode "get(0)" que l'on essaye de "stubber" est appelée sur une instance d'ArrayList réelle au moment où on essaye de la stubber, alors que la liste ne contient encore aucun élément. D'où l'exception.

La solution consiste à utiliser une autre forme de stubbing, le doReturn:
@Test
public void testDoReturnStubbingOnSpy(){
     List liste = new ArrayList();
     List spy = spy(liste);
     doReturn("hello").when(spy).get(0);
     assertEquals(spy.get(0),"hello");
}
Cette manière de stubber est totalement différente, puisque get(0) n'est pas appelé sur la liste au moment de la définition du doReturn.

On pourra aussi "stubber" une méthode qui renvoie void.
@Test
public void testDoNothingOnSpy(){
     List liste = new ArrayList();
     List spy = spy(liste);
     spy.add("hello");
     spy.add("bonjour");
     doNothing().when(spy).clear();
     spy.clear();
     assertEquals(spy.size(),2);
}
Ici, la méthode clear n'a pas été appelée sur le spy (et donc la classe réelle) lors de la déclaration du stub et donc elle n'est pas appelée réellement. Quant au stub, il ne fait rien et la liste espionnée n'est pas nettoyée.

Pour terminer, un dernier test:
@Test
public void testHowChangeEverything(){
     final List<String> liste = new ArrayList<String>();
     List<String> spy = spy(liste);
     doAnswer(new Answer<Object>(){
          public Object answer(InvocationOnMock invocation) throws Throwable {
               String s = (String)invocation.getArguments()[0];
               liste.add(s.toUpperCase());
               return null;
          }}
     ).when(spy).add(anyString());

     spy.add("bonjour");

     assertEquals(liste.get(0),"BONJOUR");
}
Ce test réécrit le comportement d'une méthode, à savoir add, pour qu'elle ajoute sur la liste espionnée, non pas la String originale, mais la String mise en majuscule.

Amusant, mais il y a d'autres moyens plus simples pour arriver au même résultat, sans passer par Mockito.

Moralité: il faut faire preuve de discernement.

Ca me rappelle ce développeur, dont les tests avec Mockito mockaient une interface, stubbaient ses méthodes et vérifiaient que les stubs renvoyait les réponses qu'ils avaient définies. Ils n'échouaient jamais...

samedi 22 février 2014

Modèle robuste: immutabilité et Value Objects

Dans l'article JavaBean, la justification du mauvais orienté objet, nous avons vu comment utiliser les constructeurs et les setters pour garantir que les valeurs passées aux objets étaient correctes, en respectant le principe essentiel que l’objet était le seul responsable de la qualité de ses données.

Cette responsabilité peut être partagée par les paramètres eux-mêmes et c’est ce que nous allons voir dans un instant.

A toute épreuve?


Mais revenons un instant sur la dernière version de l’objet Rectangle de l’article précédent. Elle est parfaitement robuste et ne permet pas la création d’un objet incohérent.

Vraiment ?

Oui, vraiment, dans des conditions normales d’utilisation.

Là... vous le voyez, le piège? Des conditions normales d’utilisation, c’est-à-dire lorsque le développeur n’utilise que l’interface que l’objet lui offre.

S'il utilise la réflexion pour accéder directement aux attributs de l’objet, rien ne peut malheureusement garantir sa cohérence, comme le montre le test (TestNg) suivant :
@Test
public void testCanCreateIncorrectRectangleByReflection(){
     Rectangle r = new Rectangle(2.0,1.0);
     try {
          Field dimension1 = Rectangle.class.getDeclaredField("dimension1");
          Field dimension2 = Rectangle.class.getDeclaredField("dimension2");
          dimension1.setAccessible(true);
          dimension2.setAccessible(true);
          dimension1.set(r, -6.0);
          dimension2.set(r, 0.0);
     } catch (SecurityException e) {
          fail("Security manager...");
     } catch (NoSuchFieldException e) {
          fail("Etonnant...");
     } catch (IllegalArgumentException e) {
          fail("Etonnant...");
     } catch (IllegalAccessException e) {
          fail("Etonnant...");
     }

     assertEquals(r.getLongueur(),0.0);
     assertEquals(r.getLargeur(),-6.0);
     assertEquals(Math.abs(r.getSurface()),0.0);
     assertEquals(r.getPerimetre(),-12.0);
}
Ces quelques lignes permettent d’avoir un rectangle avec une dimension nulle et une autre négative, et des propriétés incohérentes.

Il n’y a malheureusement rien à faire pour contrer cela. Cependant, la réflexion reste lourde à utiliser (raison pour laquelle je suis généralement contre le développement de méthodes utilitaires qui faciliteraient la réflexion). Ceux qui l’utilisent le font, on peut le supposer, en connaissance de cause et assument les risques que cela entraîne.

Immutabilité


Poursuivons notre étude des modèles robustes et examinons une notion abordée succinctement dans l’article précédent : l’immutabilité. (A noter qu’en français, immutabilité n’existe pas et que c’est immuabilité qu’il faut utiliser. Néanmoins, je continuerai d’utiliser immutabilité, proche du mot anglais d’origine « immutability ».)

L’immutabilité signifie que, une fois l’objet créé, ses propriétés ne peuvent plus être modifiées.

Ceci pourrait soulever débat : sont-ce les attributs ou les propriétés qui ne peuvent plus être modifiés. Dans l’article précédent, j’ai fait une distinction entre les attributs (champs définis dans la classe) et les propriétés (valeurs exposées par la classe, de manière standard via des setters et des getters). Un objet peut utiliser certains attributs, mais exposer de propriétés différentes.

Dès lors, l’immutabilité touche-t-elle les attributs ou les propriétés ? Si les attributs sont immutables, quel effet pourrait-on attendre de la modification des propriétés ? Aucun. Par contre, les propriétés pourraient être immutables, mais les attributs seraient modifiables par l’objet lui-même, étant donné qu’il est le seul responsable de ses attributs.

Pour couper court à la discussion, je considérerai qu’attributs et propriétés sont immutables.

L’immutabilité peut aussi toucher à certaines propriétés et non à toutes. Dans ce cas, l’objet lui-même n’est bien sûr pas immutable.

C’est l’analyse du domaine de l’application qui déterminera ce qui est immutable ou non.

Pour ce qui suit, je vais utiliser une nouvelle classe : Book. Il s’agit d’une classe qui définit un livre (normalement, il n’était pas nécessaire de passer par Google Traduction pour le comprendre) lequel a les propriétés suivantes :
  • un titre
  • un auteur
  • un numéro ISBN
L'ISBN est un numéro de codification unique, par édition. Il doit respecter certaines règles pour être valide.

Pour ce qui suit, supposons que les critères de validité des propriétés soient les suivants :
  • Le numéro ISBN doit toujours être défini et doit être valide. Une fois défini, il n’y a pas moyen de le modifier.
  • Le titre doit toujours être défini par une chaîne non vide. Une fois défini, il ne peut être modifié.
  • L’auteur est facultatif. Il peut être modifié. S’il est défini, il doit être une chaîne non vide.
Notons également que le numéro ISBN constitue une clé métier.
Pour créer cette classe, je vais de plus employer deux librairies utilitaires:
La classe Book répondant aux conditions ci-dessus est la suivante :
public class Book {
     private String isbn;
     private String title;
     private String author;

     public Book(String isbn, String title){
          this(isbn,title,null);
     }

     public Book(String isbn, String title, String author){
          ISBNValidator validator = ISBNValidator.getInstance(true);
          if(! validator.isValid(isbn)){
               throw new IllegalArgumentException("Isbn incorrect");
          }
          if(StringUtils.isBlank(title)){
               throw new IllegalArgumentException("Le titre ne peut être vide");
          }
          this.isbn = validator.validate(isbn);
          this.title = title.trim();
          this.author = StringUtils.trimToNull(author);
     }

     public void setAuthor(String author) {
          this.author = StringUtils.trimToNull(author);
     }

     public String getIsbn() {
          return isbn;
     }

     public String getTitle() {
          return title;
     }

     public String getAuthor() {
          return author;
     }

     @Override
     public int hashCode() {
          return isbn.hashCode();
     }

     @Override
     public boolean equals(Object obj) {
          if (this == obj)
               return true;
          if (obj == null)
               return false;
          if (!(obj instanceof Book))
               return false;
          Book other = (Book) obj;
          return isbn.equals(other.isbn);
     }
}
Quelques remarques :

  • L’IsbnValidator valide les ISBN-13 et ISBN-10, la valeur null étant non valide. Il va aussi les convertir (enlever les ‘-‘) et transformer les ISBN-10 en ISBN-13. Le résultat est une String de 13 chiffres.
  • Le getIsbn renvoie la String non formatée. On pourrait aussi la formater d’une manière standard (xxx-x-xxxx-xxxx-x).
  • Un "trim" est appliqué au titre et à l’auteur afin de ne pas garder des espaces inutiles à l'avant et à l'arrière.
  • Si l’auteur est vide ("      ", par exemple), la valeur de l’attribut sera null.
  • Il y a deux constructeurs, un avec un auteur, l’autre sans. A noter que l’auteur peut être null dans le premier.
  • La méthode equals porte sur le numéro ISBN  uniquement puisqu’il correspond à une clé métier (trop souvent, je vois des equals sur l’ensemble des attributs).
  • Il n'y a qu'un seul setter, sur l'auteur, lequel valide le paramètre. C'est la seule propriété mutable.
  • Si vous demandez à Eclipse de générer le code du equals/hashcode à partir de l’attribut isbn, vous obtiendrez le code suivant :
    @Override
    public int hashCode() {
         final int prime = 31;
         int result = 1;
         result = prime * result + ((isbn == null) ? 0 : isbn.hashCode());
         return result;
    }
    
    @Override
    public boolean equals(Object obj) {
         if (this == obj)
              return true;
         if (obj == null)
              return false;
         if (!(obj instanceof Book))
              return false;
         Book other = (Book) obj;
         if (isbn == null) {
              if (other.isbn != null)
                   return false;
         } else if (!isbn.equals(other.isbn))
              return false;
         return true;
    }
    
    Néanmoins, notre modèle garantissant que l'attribut isbn n’est jamais null, certains contrôles sont superflux.
Notre classe Book est robuste. Elle ne peut contenir aucune valeur incorrecte. Les propriétés non mutables ne peuvent être modifiées. C’est parfait… enfin presque.

Value Object


C’est la question piège : "si vous deviez avoir un attribut qui est un numéro de compte/un isbn/un numéro de sécurité sociale… quel type utiliseriez-vous ?". Piège, car trop souvent la réponse va être "une String" (même si "un long" aurait été pire). Une String, c’est facile d’emploi et ça peut contenir n’importe quoi… vraiment n’importe quoi.

C’est le cas de notre isbn qui est une String. Nous sommes obligés de le valider lorsqu’on le passe en paramètre du constructeur. Tant qu’on reste dans le contexte de Book, ce n’est pas vraiment un problème. Mais si isbn devient un paramètre de méthode, il faudra peut-être le valider à chaque utilisation.

L’erreur est plus fondamentale : un des atouts de Java est d’avoir un typage fort. Lorsque j’utilise un Integer, je suis sûr que c’est un nombre entier, pas un décimal, pas une chaîne de caractères. Par contre, une String pour un numéro ISBN, c’est particulièrement faible, car cette String peut contenir n'importe quoi avant d'être validée.

La solution, ce sont les Value Objects.

Littéralement, ce sont des objets qui contiennent une valeur (sans entrer dans les détails, cela ne signifie pas nécessairement qu’ils n’ont qu’un seul attribut ou une seule propriété).

Integer est un exemple de Value Object, un objet qui contient un entier comme valeur. String aussi (objet qui contient une chaîne de caractères, quoi qu’elle représente).

Le numéro ISBN pourrait être avantageusement remplacé par un Value Object.

Avant de montrer le code, voyons quelques propriétés des Value Objects :
  • La valeur contenue dans le Value Object est toujours correcte (cohérence de l’objet). Essayez new Integer("toto"), vous m’en direz des nouvelles.
  • Un Value Object est généralement immutable. String, Integer, BigDecimal sont tous immutables (c’est même une erreur commune de croire le contraire).
  • On pourrait s’attendre à ce que deux Value Objects contenant la même valeur soient en fait la même référence. C’est rarement le cas, car c’est très difficile à implémenter (sauf si le nombre de valeurs différentes est petit).
Tenant compte de ces remarques, voici une classe pour les Value Objects ISBN :
public class Isbn {
     private String isbn;

     public Isbn(String isbn){
          if(!validate(isbn)){
               throw new IllegalArgumentException("Isbn incorrect");
          }
          this.isbn = ISBNValidator.getInstance(true).validate(isbn);
     }

     public Isbn(Isbn isbn){
          this.isbn = isbn.isbn;
     }

     public static boolean validate(String isbn){
          return ISBNValidator.getInstance(true).isValid(isbn);
     }

     public String getIsbn() {
          return isbn;
     }

     @Override
     public int hashCode() {
          return isbn.hashCode();
     }

     @Override
     public boolean equals(Object obj) {
          if (this == obj)
               return true;
          if (obj == null)
               return false;
          if (!(obj instanceof Isbn))
               return false;
          Isbn other = (Isbn) obj;

          return isbn.equals(other.isbn);
     }
}
Quelques points notables :
  • L’objet reste construit à partir d’une String. Mais cette String n’est utilisée qu’une seule fois, comme paramètre du construteur. Elle peut être une valeur entrée par l’utilisateur. Néanmoins, c’est le seul endroit où une String représentera un numéro ISBN. Une fois l’objet Isbn construit, plus aucune méthode une String pour un ISBN. Ceci dit, au sein de l'objet Isbn, le numéro est encodé dans une String.
  • Le deuxième constructeur n’est pas absolument nécessaire. C’est un constructeur de copie, parfois utile.
  • La méthode validate, statique, permet de valider la chaîne de caractère supposée contenir un numéro ISBN. Elle évite de devoir faire un try-catch autour de la construction du Value Object. Au lieu de :
    try {
         isbn = new Isbn(une string);
    } catch(IllegalArgumentException e) {
         //Gérer le fait que la String n'est pas valide
    }
    
    on écrira :
    if(Isbn.validate(une string)){
         isbn = new Isbn(une string);
    } else {
         //Gérer le fait que la String n'est pas valide
    }
    
    Bien sûr, la validation sera faite deux fois, mais l’écriture est plus propre.
  • Pour la méthode equals notez que l’attribut isbn n’est jamais null.
  • Une partie de la logique que l’on trouvait dans Book est maintenant dans Isbn.
Le code de Book devient :
public class Book {
     private Isbn isbn;
     private String title;
     private String author;

     public Book(Isbn isbn, String title){
          this(isbn,title,null);
     }

     public Book(Isbn isbn, String title, String author){
          if(isbn==null){
               throw new IllegalArgumentException("Isbn doit être fourni");
          }
          if(StringUtils.isBlank(title)){
               throw new IllegalArgumentException("Le titre ne peut être vide");
          }
          this.isbn = isbn;
          this.title = title.trim();
          this.author = StringUtils.trimToNull(author);
     }

     public void setAuthor(String author) {
          this.author = StringUtils.trimToNull(author);
     }

     public Isbn getIsbn() {
          return isbn;
     }

     public String getTitle() {
          return title;
     }

     public String getAuthor() {
          return author;
     }

     @Override
     public int hashCode() {
          return isbn.hashCode();
     }

     @Override
     public boolean equals(Object obj) {
          if (this == obj)
               return true;
          if (obj == null)
               return false;
          if (!(obj instanceof Book))
               return false;
          Book other = (Book) obj;
          return isbn.equals(other.isbn);
     }
}
Le résultat n’est pas beaucoup plus court qu’auparavant, mais à présent l’objet Isbn s’occupe de sa propre cohérence.

Malheureusement, nous devons toujours vérifier que la référence passée en paramètre pour isbn n’est pas nulle.

Jusqu’où aller ?


Ne serait-il pas intéressant d’utiliser un Value Object pour le titre, ce qui garantirait qu’il y a un contenu ? Dans le premier exemple de JavaBean, la justification du mauvais orienté objet, dans les personnes à charge, ne serait-il pas intéressant d’avoir un StrictlyPositiveInteger qui encapsulerait un entier obligatoirement strictement positif ?

Il n’y a pas de réponse catégorique à ces questions. Le point important à considérer est la réutilisation du Value Object : si la classe est peu utilisée hors d’un contexte spécifique (c'est-à-dire, un autre objet), un Value Object a peu d’intérêt.

Malgré tout, on pourrait être tenté d’utiliser des Value Object partout, mais cela demande du travail supplémentaire. D’autant plus que, comme nous le verrons une prochaine fois, il faudra mettre la main à la pâte pour faire fonctionner ces objets avec Hibernate.

lundi 17 février 2014

Traitements asynchrones avec Spring @Async

Dans une application Web, l'utilisateur ne doit pas attendre inutilement. Si une requête HTTP déclenche un traitement long et que le résultat de ce traitement n’a aucun impact sur la réponse à renvoyer, pourquoi attendre qu'il se termine ?

Un traitement long ne devrait jamais être appelé de manière synchrone. Cependant, le rendre asynchrone demande un travail plus ou moins important. Heureusement, Spring vient à la rescousse avec une annotation, @Async, qui fait tout le boulot.

Traitements asynchrones


L'exemple ci-dessus parle d'une requête Web, mais n'importe quel processus peut tirer parti de l'asynchronisme. Web ou non, il y a deux cas de figure:
  • Le premier, c’est celui évoqué ci-dessus : un processus (peut-être une requête Web) entraîne un traitement dont l’issue lui importe peu. On peut imaginer un log d’accès à des fins de statistiques. Ce n’est pas nécessairement un long traitement, mais le processus parent peut suivre son cours, indépendamment de son résultat (le log). Ce besoin peut être implémenté de différentes manières, mais le rendre asynchrone, c’est-à-dire créer un thread séparé du processus parent et y faire s’exécuter le traitement, est une solution.
  • Le deuxième cas de figure est que le résultat d'un processus dépend du résultat d’un long traitement. Il est intéressant de lancer ce dernier de manière asynchrone car, pendant qu’il s’exécute dans son thread, le processus principal peut suivre son cours en parallèle. Ce système est plus difficile à mettre en place et implique typiquement l’utilisation d’un Future, un objet qui, comme son nom l’indique, contiendra une réponse dans le futur.

Spring @Async


Avec Spring, il suffit de placer l'annotation @Async sur la méthode englobant le traitement long pour le rendre asynchrone. Bon, c’est vrai, il faut un peu de configuration aussi.

Pour commencer, Spring aura besoin d’un TaskExecutor qui se chargera de l’exécution asynchrone de la méthode. Spring propose plusieurs implémentations de cette interface, que je ne vais pas examiner ici. Il suffit de savoir que, dans la plupart des cas, sa création sera très simple.

Dans le fichier xml de configuration de Spring, il suffit d’indiquer:
<beans xmlns="http://www.springframework.org/schema/beans"
    xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
    xmlns:task="http://www.springframework.org/schema/task"
    xsi:schemaLocation="http://www.springframework.org/schema/beans http://www.springframework.org/schema/beans/spring-beans-3.0.xsd
    http://www.springframework.org/schema/task http://www.springframework.org/schema/task/spring-task-3.0.xsd" 
    default-autowire="byName">

    <task:executor id="executor" pool-size="10"/>
    <task:annotation-driven executor="executor" />

    <!--A suivre…-->
Cette configuration définit un bean nommé "executor", instance de la classe ThreadPoolTaskExecutor (un pool de 10 threads dans ce cas).

Il faut ensuite activer la définition de l'asynchronisme via annotation, en précisant le TaskExecutor à utiliser, dans la ligne <task:annotation-driven />.

A noter que cette dernière entrée permet aussi de configurer des tâches planifiées (des Schedulers, avec l’annotation @Scheduled). Il n’en sera pas question ici.

Ecriture de services asynchrones


Pour continuer notre exploration, je vais utiliser une méthode de longue : le calcul d’une factorielle utilisant des BigInteger.

Quelques remarques sur ce choix et sur son implémentation :

  • Pour le moment, sur une machine standard, ce traitement est "long". Je ne sais pas si ce sera toujours le cas dans quelques années.
  • Le calcul de la factorielle sert généralement de démonstration à la récursivité. Néanmoins, l’implémentation récursive est un peu particulière dans le cas présent. J’y reviendrai plus tard, mais dans l’immédiat, le traitement sera non récursif.
  • Le bean contenant le calcul de la factorielle implémentera une interface. C’est effectivement une bonne pratique de masquer l’implémentation, d’autant plus que Spring doit créer un proxy pour gérer l'asynchronisme, un proxy implémentant l'interface et interceptant les appels vers le véritable bean. Ceci étant dit, la création du proxy fonctionne également sans interface… tant que la classe n’est pas "final".
  • Enfin, pour cette démonstration, j’ai choisi un traitement qui retourne une valeur. Cela implique donc l’utilisation d’un Future. Comment faire si aucun retour n’est attendu ? Renvoyer void tout simplement.

L’interface est simple :
public interface AsyncBean {
     Future<BigInteger> asyncFact(BigInteger n);
}
A noter que la méthode renvoie un Future<BigInteger>. La réponse sera un BigInteger, mais dans le futur…

Comment gère-t-on un Future ? Plusieurs méthodes de Future<T> sont utiles :

  • isDone renvoie true si le résultat est disponible;
  • get() permet de récupérer la valeur (le résultat) du Future. Cette fonction est synchrone : elle attend que le résultat soit disponible. Il est possible d’y ajouter un timeout. Dans ce cas, si la réponse n’est pas disponible dans le délai imparti, une exception est lancée;
  • cancel() permet d’annuler la tâche;
  • isCancelled permet de vérifier si la tâche a été annulée.

Une implémentation asynchrone (et non récursive) de la méthode est la suivante :
@Component
public class AsyncBeanImpl implements AsyncBean {
 
     @Async
     public Future<BigInteger> asyncFact(final BigInteger n) {
          BigInteger accu = BigInteger.ONE;
          BigInteger counter = BigInteger.ONE;
          while(counter.compareTo(n) != 1){
               accu = accu.multiply(counter);
               counter = counter.add(BigInteger.ONE);
          }
          return new AsyncResult<BigInteger>(accu);
     }
}
C’est grâce à l’annotation @Async que l’appel de la méthode sera asynchrone. Quant à l’annotation @Component, elle fera de notre classe un bean Spring.

La valeur de retour est un AsyncResult, une implémentation Spring de Future. Elle est remplie avec le résultat du calcul de la factoriel.

On peut tester l’implémentation avec la classe de test (TestNg) suivante :
@ContextConfiguration(locations="classpath:async/test-async-spring.xml")
public class TestAsyncExecution extends AbstractTestNGSpringContextTests{
     @Autowired
     private AsyncBean asyncBean;

     @Test
     public void testCalculationIsRealyAsynchronous(){
          //La valeur attendue, soit 1000!, il fallait bien un BigInteger
          BigInteger response = new BigInteger("402387260077093773543702433923003985719374864210714632543799910429938512398629020592044208486969404800479988610197196058631666872994808558901323829669944590997424504087073759918823627727188732519779505950995276120874975462497043601418278094646496291056393887437886487337119181045825783647849977012476632889835955735432513185323958463075557409114262417474349347553428646576611667797396668820291207379143853719588249808126867838374559731746136085379534524221586593201928090878297308431392844403281231558611036976801357304216168747609675871348312025478589320767169132448426236131412508780208000261683151027341827977704784635868170164365024153691398281264810213092761244896359928705114964975419909342221566832572080821333186116811553615836546984046708975602900950537616475847728421889679646244945160765353408198901385442487984959953319101723355556602139450399736280750137837615307127761926849034352625200015888535147331611702103968175921510907788019393178114194545257223865541461062892187960223838971476088506276862967146674697562911234082439208160153780889893964518263243671616762179168909779911903754031274622289988005195444414282012187361745992642956581746628302955570299024324153181617210465832036786906117260158783520751516284225540265170483304226143974286933061690897968482590125458327168226458066526769958652682272807075781391858178889652208164348344825993266043367660176999612831860788386150279465955131156552036093988180612138558600301435694527224206344631797460594682573103790084024432438465657245014402821885252470935190620929023136493273497565513958720559654228749774011413346962715422845862377387538230483865688976461927383814900140767310446640259899490222221765904339901886018566526485061799702356193897017860040811889729918311021171229845901641921068884387121855646124960798722908519296819372388642614839657382291123125024186649353143970137428531926649875337218940694281434118520158014123344828015051399694290153483077644569099073152433278288269864602789864321139083506217095002597389863554277196742822248757586765752344220207573630569498825087968928162753848863396909959826280956121450994871701244516461260379029309120889086942028510640182154399457156805941872748998094254742173582401063677404595741785160829230135358081840096996372524230560855903700624271243416909004153690105933983835777939410970027753472000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000");
          Future<BigInteger> r = asyncBean.asyncFact(BigInteger.valueOf(1000L));
          assertFalse(r.isDone(),"Pas encore calculé");
          BigInteger result = null;
          try{
               result = r.get();
          } catch(Exception e){
               fail("Exception durant l’exécution", e);
          }
          assertNotNull(result,"Le résultat ne peut être null ");
          assertTrue(r.isDone(),"Maintenant, la tâche est terminée ");
          assertEquals(result, response,"Le résultat doit évidemment être correct");
     }
}
Le fichier test-async-spring.xml est le suivant :
<beans xmlns="http://www.springframework.org/schema/beans"
    xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
    xmlns:context="http://www.springframework.org/schema/context"
    xmlns:task="http://www.springframework.org/schema/task"
    xsi:schemaLocation="http://www.springframework.org/schema/beans http://www.springframework.org/schema/beans/spring-beans-3.0.xsd
    http://www.springframework.org/schema/context http://www.springframework.org/schema/context/spring-context-3.0.xsd
    http://www.springframework.org/schema/task http://www.springframework.org/schema/task/spring-task-3.0.xsd" 
    default-autowire="byName">

    <context:component-scan base-package="*" />

    <!-- Crée un ThreadPoolTaskExecutor -->
    <task:executor id="executor" pool-size="10"/>

    <task:annotation-driven executor="executor" />

</beans>
L’implémentation peut paraître curieuse car l’instanciation du Future ne sera faite qu’après le calcul. Pourtant, dans notre test, nous recevons bien un Future "tout de suite", alors que notre méthode n’a pas encore terminé son calcul et donc créé son Future avec un résultat.

En fait, Spring utilise l’AOP pour décorer notre classe avec un AsyncExecutionInterceptor, lequel crée un Callable et le soumet au TaskExecutor (qui gère les threads et les exécutions). Cette soumission renvoie un Future qui ne sera initialisé que lors du retour de notre méthode en prenant la valeur de notre Future.

Il est assez simple de vérifier (en mode debug) que le Future reçu par le test, lors de l’appel asyncBean.asyncFact(BigInteger.valueOf(1000L)) n’est pas le même que celui renvoyé par notre implémentation : le premier est une java.util.concurrent.FutureTask (renvoyée par l'AsyncExecutionInterceptor), alors que le deuxième, que nous avons instancié, est org.springframework.scheduling.annotation.AsyncResult.

Au final, le Future renvoyé par notre méthode n’est pas si « futur » que ça puisqu’il est créé directement avec la valeur de retour. Mais il est obligatoire de renvoyer un Future. Si ce n’était pas le cas et que la méthode renvoyait un BigInteger directement, l’intercepteur de Spring renverrait null et jamais la bonne valeur.

La version récursive


La version récursive n’est pas complexe, si ce n’est que sa valeur de retour doit être un Future. On a donc une implémentation comme suit :
@Async
public Future<BigInteger> asyncRecursiveFact(final BigInteger n) throws InterruptedException, ExecutionException {
     if(n.equals(BigInteger.ZERO)){
          return new AsyncResult<BigInteger>(BigInteger.ONE);
     } else {
     return new AsyncResult<BigInteger>(n.multiply(asyncRecursiveFact(n.subtract(BigInteger.ONE)).get()));
     }
}
Elle fonctionne correctement et est beaucoup plus rapide que la première. Mais elle est un peu lourde.

A chaque itération, un objet Future est créé. Comme nous l’avons vu, ce n’est qu’un container, initialisé dès sa création avec une valeur. Il n’a donc aucun intérêt, si ce n’est de surcharger l’écriture de la méthode : récupération de la valeur par un get(), gestion des exceptions (ici simplement transmises dans un throws, mais ce n'est pas très propre).

C’est pourquoi je préfère la deuxième implémentation, plus claire (et aussi 20% plus rapide que la précédente) :
@Async
public Future<BigInteger> asyncRecursiveFactOther(final BigInteger n) {
     return new AsyncResult<BigInteger>(fact(n));
}
 
private BigInteger fact(BigInteger n){
     if(n.equals(BigInteger.ZERO)){
          return BigInteger.ONE;
     } else {
          return n.multiply(fact(n.subtract(BigInteger.ONE)));
     }
}
Le calcul récursif se fait sans utiliser les Future, mais est appelé de manière asynchrone. L’objet Future n’est créé qu'à la fin, que lorsque le calcul est terminé.

Code source


Comme expliqué dans l’article "Deux projets prototypes et didactiques", les sources de cet article sont disponibles sur GitHub :

  • les beans sont ici
  • la classe de test est ici 
  • le fichier xml de configuration ici


vendredi 14 février 2014

JavaBean: la justification du mauvais orienté objet

Il y a quelques semaines, je découvrais le code suivant (à noter que les extraits de code présentés ici ne sont pas les codes réels, cependant les principes restent les mêmes) :
public class Situation {
     private int enfantsACharge;
     private int autresACharge;
     private boolean personneACharge;

     public int getEnfantsACharge() {
          return enfantsACharge;
     }
     public void setEnfantsACharge(int enfantsACharge) {
          this.enfantsACharge = enfantsACharge;
     }
     public int getAutresACharge() {
          return autresACharge;
     }
     public void setAutresACharge(int autresACharge) {
          this.autresACharge = autresACharge;
     }
     public boolean isPersonneACharge() {
          return personneACharge;
     }
     public void setPersonneACharge(boolean personneACharge) {
          this.personneACharge = personneACharge;
     }
}

Cette classe décrit la situation fiscale d’une personne, à savoir le nombre d’enfants à charge, le nombre d’autres personnes à charge et si elle a ou non des personnes à charge. Rien ne vous choque ?

Non? Continuons…

Un peu plus loin, un service :
public class SituationService {
     public Situation createSituation(int enfantsACharge, int autresACharge){
          Situation situation = new Situation();
          situation.setEnfantsACharge(enfantsACharge);
          situation.setAutresACharge(autresACharge);
          situation.setPersonneACharge(enfantsACharge !=0 || autresACharge != 0);
          return situation;
     }
}
Toujours rien ne vous choque ?

Si vous répondez une nouvelle fois non, c’est que les JavaBeans ont eu raison de vous et que le domaine anémique (Anemic Domain Model) est votre quotidien. Vous ne faites donc plus de l’orienté objet, mais un ersatz procédural dans lequel les objets sont, soit des conteneurs de données, soit des conteneurs de méthodes. Bref, de la mauvaise programmation orientée objet.

Voici une version plus correcte et plus orientée objet de la situation fiscale :
public class Situation {
     private int enfantsACharge;
     private int autresACharge;

     public void setEnfantsACharge(int enfantsACharge) {
          if(enfantsACharge < 0){
               throw new IllegalArgumentException("Le nombre d'enfants à charge doit être positif");
          }
          this.enfantsACharge = enfantsACharge;
     }
     public int getEnfantsACharge() {
          return enfantsACharge;
     }
     public void setAutresACharge(int autresACharge) {
          if(autresACharge < 0){
               throw new IllegalArgumentException("Le nombre d'autres personnes à charge doit être positif");
          }
          this.autresACharge = autresACharge;
     }
     public int getAutresACharge() {
          return autresACharge;
     }
     public boolean isPersonnesACharge(){
          return enfantsACharge > 0 && autresACharge > 0;
     }
}
Quant à la méthode createSituation de SituationService, elle disparaît du service (lequel a peut-être d'autres raisons, légitimes, d'exister).

Cette version apporte quelques améliorations :

  • Il est impossible de passer un nombre négatif pour les enfants ou les autres à charge. Les setters vérifient la qualité des paramètres et ne les acceptent que s’ils sont corrects.
  • Le statut "personnesACharge" est déterminé par l’objet lui-même. Avant, sa valeur dépendait d’un calcul extérieur et rien ne garantissait qu’il soit calculé correctement. Accessoirement, l’attribut "personnesACharge" a disparu. Il reste juste un accesseur (qui suit la standard JavaBean, hé oui…).
Au final, ce modèle est plus robuste que le premier. Il n’y a plus moyen de créer un objet Situation qui n’aurait aucune cohérence (3 enfants à charge, mais personnesACharge false). Et qu’on ne vienne pas me dire que le service permettait d'en faire autant, puisque l’objet pouvait être créé à l'extérieur du service.

La première fois que j’ai présenté cette approche à un collègue, il s’est écrié : « si tu fais ça, tu vas casser le contrat JavaBean ».

A cela, je réponds deux choses :
  1. "oui, et alors ?" parce que les JavaBeans ne sont pas une religion (encore moins un contrat) et que les frameworks qui prétendent en avoir besoin (Hibernate, Spring) mentent un peu (je verrai ça une autre fois).
  2. "de toute façon, la classe Situation du départ n’était pas non plus un JavaBean" : elle n’implémente pas Serializable (exercice: comptez le nombre de vos entités Hibernate/Jpa, soit disant des JavaBeans, qui implémentent Serializable).

Modèle robuste

La propriété la plus importante de l’orienté objet, c’est l’encapsulation : l’objet est le seul responsable de la qualité de ses attributs et décide seul des informations et des opérations qu’il veut exposer vers l’extérieur.

Pour continuer ma démonstration, je vais utiliser une classe plus simple, mais mal écrite : un rectangle.
public class Rectangle {
     private double longueur;
     private double largeur;

     public double getLongueur() {
          return longueur;
     }
     public void setLongueur(double longueur) {
          this.longueur = longueur;
     }
     public double getLargeur() {
          return largeur;
     }
     public void setLargeur(double largeur) {
          this.largeur = largeur;
     }
}
Soyons sérieux ! Ce n’est pas de l’orienté objet. Pourtant, nombreux sont ceux qui s'imaginent respecter l'encapsulation parce que les attributs sont private...

Récemment, un développeur me disait, un peu dépité, "je ne vois pas pourquoi on continue de mettre les attributs en privé si de toute façon les setters et les getters permettent de les modifier directement". Il avait raison !

Dans le cas présent, un objet Rectangle n’a aucun contrôle sur la qualité des propriétés qui lui sont injectées.
Rectangle r = new Rectangle() ;
r.setLongueur(-3.0) ; //Non, mais allô quoi...
C’est absurde. Sans pousser trop l’analyse du domaine, il est clair qu’un rectangle ne peut avoir une longueur ou une largeur négative, ni même égale à 0.

Là où je suis le plus étonné, c'est que de nombreux développeurs pensent que la spécification JavaBean leur impose d’avoir des setters ridicules.

Première étape

Nous commencerons donc par consolider notre modèle du domaine en réécrivant les setters de manière intelligente :
public class Rectangle {
       private double longueur;
       private double largeur;
       public double getLongueur() {
             return longueur;
       }
       public void setLongueur(double longueur) {
             if(longueur <= 0){
                    throw new IllegalArgumentException("La longueur doit être strictement positive");
             }
             this.longueur = longueur;
       }
       public double getLargeur() {
             return largeur;
       }
       public void setLargeur(double largeur) {
             if(largeur <= 0){
                    throw new IllegalArgumentException("La largeur doit être strictement positive");
             }
             this.largeur = largeur;
       }
}
Impossible à présent d'injecter des valeurs incorrectes dans notre objet, les setters y veillent.

Le choix de l'exception est logique. C'est une RuntimeException pour laquelle on ne fait normalement aucun try-catch. Si cette erreur arrive, c'est que le développeur n'a pas été vigilant: avant d'utiliser les paramètres, il devait les valider. Lorsqu'elle arrive, il n'y a rien qu'on puisse faire. Il fallait agir avant. De la même manière qu'on ne fait pas de try-catch pour des NullPointerException, mais qu'on vérifie les références suspectes en les comparant à null.

Cette classe ne permet pas d'injecter dans ses instances des valeurs incorrectes.

En fait, ce n'est pas tout à fait exact puisqu'une classe qui hériterait de Rectangle pourrait surcharger les setters et les réécrire sans la validation. La réplique à ce risque consiste à mettre les setters en final.

En dehors de ce risque, peut-on considérer qu'une instance de Rectangle sera désormais correcte?

Deuxième étape

La réponse à la question précédente est hélas! non, comme le démontre le code suivant:
Rectangle r = new Rectangle();
r.setLongueur(3.0);
System.out.println(r.getLargeur());
Réponse... 0 ! Soit une largeur inacceptable. L'objet n'est pas cohérent.

Moralité, si certaines propriétés doivent obligatoirement être remplies, la création de l'objet doit être atomique.

Le moyen le plus simple pour obtenir cette atomicité est de passer par un constructeur.
public class Rectangle {
       private double longueur;
       private double largeur;
       
       public Rectangle(double longueur, double largeur){
             if(longueur <= 0){
                    throw new IllegalArgumentException("La longueur doit être strictement positive");
             }
             if(largeur <= 0){
                    throw new IllegalArgumentException("La largeur doit être strictement positive");
             }
             this.largeur = largeur;
             this.longueur = longueur;
       }
       public double getLongueur() {
             return longueur;
       }
       public double getLargeur() {
             return largeur;
       }
       public double getSurface(){
             return longueur * largeur;
       }
}
Là, plus moyen de se tromper. Lorsqu'il est créé, un rectangle a une longueur et une largeur, toutes deux strictement positives.

J'ai aussi introduit un autre aspect: l'objet est immutable. Une fois le rectangle créé avec une longueur et une largeur données, aucune de ses deux dimensions ne peut être modifiées. C'est le domaine de l'application qui déterminera si l'immutabilité est une propriété de l'objet ou non. Il est également possible que seules certaines propriétés soient immutables et pas les autres. D'une manière générale, je trouve que les développeurs ne font pas assez attention à l'immutabilité et que leur modèle gagnerait en qualité si c'était le cas.

Si le Rectangle devait avec une longueur mutable, il suffirait d'y ajouter une méthode setLongueur, qui vérifie la qualité du paramètre et qui permet de modifier l'attribut longueur précédemment fixé dans le constructeur.

Parfait? Presque, mais on peut mieux faire...

Troisième étape

Vous allez dire que je chicane (et c'est certainement vrai), mais rien ne m'empêche de construire un rectangle où la longueur est plus courte que la largeur.

Allez, un petit dernier:
public class Rectangle {
       private double dimension1;
       private double dimension2;
       
       public Rectangle(double dimension1, double dimension2){
             if(dimension1 <= 0 || dimension2 <= 0){
                    throw new IllegalArgumentException("Les dimensions doivent être strictement positives");
             }
             this.dimension2 = dimension1;
             this.dimension1 = dimension2;
       }
       public double getLongeur() {
             return dimension1>dimension2?dimension1:dimension2;
       }
       public double getLargeur() {
             return dimension1>dimension2?dimension2:dimension1;
       }
       public double getSurface(){
             return dimension1 * dimension2;
       }
}
Maintenant, la longueur est plus grande que la largeur. Ce qui est intéressant avec cette manière de procéder, c'est qu'elle montre bien le principe d'encapsulation. L'objet expose certaines propriétés (avec des getters, puisque c'est le standard JavaBean): sa longueur, sa largeur, sa surface et je pourrais y ajouter son périmètre. Mais ces propriétés ne correspondent en fait à aucun attribut de l'objet, lequel fait ce qu'il veut tant qu'il présente une interface cohérente.

Il y a moyen d'améliorer cette classe: on peut lui ajouter un equals (et ceux qui croient qu'il suffit de vérifier l'égalité des propriétés se planteront), un hashcode, un toString...

Un détail peut éventuellement déranger (en particulier si comme moi vous avez fait du C++ où c'est considéré comme une erreur): le constructeur lance une exception. Ce n'est pas un problème en Java.

Factory et builder

Il n'y a pas besoin d'améliorer la robustesse de ce Rectangle. La construction à l'aide des constructeurs est suffisante. Mais il existe deux autres patterns de construction qui permettent éventuellement de renforcer le modèle: la factory et le builder.

Une factory est utile pour créer une instance d'une des nombreuses implémentations d'une classe, que l'implémentation est choisie en fonction des paramètres de création et que l'on souhaite masquer l'implémentation réellement utilisé. Un builder est quant à lui intéressant lorsqu'on est amené à créer de nombreux constructeurs pour un objet, parce qu'il y a diverses combinaisons de propriétés possibles, ou que plusieurs paramètres peuvent être ignorés ou nulls. Le builder permet aussi d'avoir une interface "fluent" pour la création d'un objet.

Attention cependant qu'aucun des deux patterns ne doit prendre la place de la solidité du modèle. Ce n'est pas parce qu'une factory garantit la création atomique d'un objet cohérent qu'il doit être possible de créer un objet incohérent en se passant de la factory (c'est le problème posé par le service au début). Le modèle doit être solide par lui même.

Ni l'un ni l'autre n'a beaucoup de sens ici. Je pourrais éventuellement créer un version euclidienne du rectangle et une version... non-euclidienne, mais je me vais me contenter de pousser le raisonnement de la factory dans un design que je trouve finalement assez élégant.

Comme je veux masquer l'implémentation, je réduis le rectangle à une interface qui expose ses propriétés via de getters.
public interface Rectangle {
       double getLongeur();
       double getLargeur();
       double getSurface();
       double getPerimetre();
}
Et je laisse la factory créer l'implémentation à l'aide d'une inner classe anonyme...
public class RectangleFactory {
       public static Rectangle create(final double dimension1,final double dimension2){
             if(dimension1 <= 0 || dimension2 <= 0){
                    throw new IllegalArgumentException("Les dimensions doivent être strictement positives");
             }

             Rectangle r = new Rectangle(){
                    private double longueur = dimension1>dimension2?dimension1:dimension2;
                    private double largeur = dimension1>dimension2?dimension2:dimension1;
                    
                    public double getLongeur() {
                           return longueur;
                    }

                    public double getLargeur() {
                           return largeur;
                    }

                    public double getSurface() {
                           return longueur*largeur;
                    }

                    public double getPerimetre() {
                           return 2*(longueur+largeur);
                    }
                    
             };
             
             return r;
       }
}
Encore une fois, le rectangle obtenu est robuste: longueur et largeur strictement positives, longueur plus grande que la largeur.

Tiré par les cheveux? Certes, mais la Factory n'aurait pas eu d'intérêt si j'avais pu créer le rectangle par d'autres moyens. En en faisant une inner class, c'est presque totalement impossible (rien n'empêche en fait un développeur de créer une implémentation non robuste de mon interface...).

Ce qu'il faut en retenir

La spécification JavaBean a été écrite pour faciliter l'utilisation de langages script (comme dans les jsp). Si "r" représente une instance de Rectangle, alors en langage scripté "r.longueur" sortira sa longueur. Grâce à la spécification, le langage scripté sait qu'il doit trouver l'information, non pas dans un attribut longueur (qui n'existe peut-être pas), mais grâce à une méthode getLongueur qui renvoie la valeur d'une propriété, laquelle peut venir directement d'un attribut ou être calculée.

Rien dans la spécification JavaBean n'oblige :
  • à avoir systématiquement un setter et un getter pour chaque attribut
  • que les setters ou les getters correspondent à des attributs réels
  • que les setters ou les getters soient écrits de la manière la plus basique (stupide?) possible
  • qu'il n'y ait aucun constructeur avec paramètres (par contre il en faut obligatoirement un sans paramètre, ce qui n'est pas le cas ici)
  • à n'avoir aucune autre méthode que les setters et les getters (si, si... j'ai déjà entendu ça comme justification de l'absence d'une méthode equals)
Dans de prochains articles, je vous montrerai d'autres moyens de construire un modèle robuste, mais aussi qu'un framework comme Hibernate, pour lequel il est "bien connu" que les entités doivent être des JavaBeans, n'en a pas du tout besoin et fonctionne très bien avec un modèle robuste (et quelques points d'attention).

Les JavaBeans ne sont pas le mal absolu et ils ont leur utilité. Mais comme pour toute chose en programmation, il est important de comprendre ce que l'on fait et pourquoi.

L'automatisme dans la création d'une classe qui consiste à écrire les attributs en private et demander à Eclipse de générer automatiquement les getters et setters est une absurdité.

On peut rarement transiger avec la qualité du modèle.