Le code du programme est écrit de haut en bas. Les instructions sont lues et exécutées dans le même ordre. C'est logique et tout le monde y est habitué depuis longtemps. Dans certains cas, vous pouvez modifier l'ordre des opérations. Mais parfois, la séquence d'appels de fonction est importante, bien que syntaxiquement cela ne soit pas évident. Il s'avère que le code semble fonctionner, même après avoir réorganisé les appels, et le résultat est inattendu.
Une fois, un code similaire a attiré mon attention.
Le problème
Une fois, fouillant dans le code de quelqu'un d'autre dans un projet commun, j'ai découvert une fonction comme:
public Product fill(Product product, Images images, Prices prices, Availabilities availabilities){ priceFiller.fill(product, prices);
Bien sûr, pas le style le plus "élégant" n'attire votre attention: les classes stockent des données (POJO), les fonctions changent les objets entrants ...
En général, cela ne semble rien. Nous avons un objet dans lequel il n'y a pas assez de données, et il y a des données elles-mêmes qui proviennent d'autres sources (services), que nous allons maintenant placer dans cet objet pour le compléter.
Mais il y a quelques nuances.
- Je n'aime pas le fait qu'aucune fonction ne soit écrite dans le style de FP et ne modifie l'objet passé en argument.
- Mais disons que cela a été fait pour réduire le temps de traitement et le nombre d'objets créés.
- Seul un commentaire dans le code indique que la séquence d'appels est importante, et vous devez être prudent lors de l'intégration du nouveau
Filler
- Mais le nombre de personnes travaillant sur le projet est supérieur à 1 et tout le monde ne connaît pas cette astuce. Surtout de nouvelles personnes dans l'équipe (pas nécessairement dans l'entreprise).
Le dernier point m'a particulièrement gardé. Après tout, l'API est construite de telle manière que nous ne savons pas ce qui a changé dans l'objet après avoir appelé cette fonction. Par exemple, avant et après, nous avons une méthode Product::getImages qui, avant d'appeler la fonction fill , produira une liste vide, puis une liste avec des images de notre produit.
Avec Filler choses sont encore pires. AvailabilityFiller ne précise pas qu'il s'attend à ce que les informations sur le prix des marchandises soient déjà intégrées dans l'objet transféré.
J'ai donc réfléchi à la manière de protéger mes collègues de l'utilisation erronée des fonctions.
Solutions proposées
J'ai d'abord décidé de discuter de ce cas avec mes collègues. Malheureusement, toutes les solutions qu'ils ont proposées ne me semblent pas être la bonne approche.
Runtimeexception
L'une des options proposées était: et vous écrivez dans le AvailabilityFiller au début de la fonction Objects.requireNonNull(product.getPrices) , puis tout programmeur recevra déjà une erreur lors des tests locaux.
- mais le prix peut vraiment ne pas être là, si le service n'était pas disponible ou une autre erreur, alors le produit devrait recevoir le statut "en rupture de stock". Nous devrons attribuer toutes sortes d'indicateurs ou quoi que ce soit pour distinguer "aucune donnée" de "même pas demandé".
- Si vous lancez une exception dans
getPrices lui-même, nous créerons les mêmes problèmes que Java moderne avec des listes
- Supposons qu'une liste soit passée à une fonction qui propose la méthode get dans son API ... Je sais que vous n'avez pas besoin de modifier les objets transférés, mais créez-en de nouveaux. Mais l'essentiel est que l'API nous propose une telle méthode, mais au moment de l'exécution, une erreur peut se produire s'il s'agit d'une liste immuable, telle que celle obtenue à partir de Collectors.toList ()
- si
AvailabilityFiller est utilisé par quelqu'un d'autre, le programmeur qui a écrit l'appel ne comprendra pas immédiatement le problème. Seulement après le lancement et le débogage. Ensuite, il doit encore comprendre le code pour savoir où obtenir les données.
Test
"Et vous écrivez un test qui se cassera si vous changez l'ordre des appels." c'est-à-dire si tous les Filler retournent un «nouveau» produit, quelque chose comme ceci se révélera:
given(priceFillerMock.fill(eq(productMock), any())).willReturn(productWithPricesMock); given(availabilityFillerMock.fill(eq(productMockWithPrices), any())).willReturn(productMockWithAvailabilities); given(imageFillerMock.fill(eq(productMockWithAvailabilities), any())).willReturn(productMockWithImages); var result = productFiller.fill(productMock, p1, p2, p3); assertThat("unexpected return value", result, is(productMockWithImages));
- Je n'aime pas les tests qui sont si "White-Box"
- Rompe avec chaque nouveau
Filler - Il se casse lors du changement de la séquence d'appels indépendants
- Encore une fois, cela ne résout pas le problème de la réutilisation de
AvailabilityFiller lui-même
Propres tentatives pour résoudre le problème
Idée
Je pense que vous avez déjà deviné que j'aimerais résoudre le problème au niveau de la compilation. Eh bien, pourquoi ai-je besoin, demande-t-on, d'un langage compilé avec un typage fort si je ne peux pas empêcher l'erreur.
Et je me demandais si un objet sans données supplémentaires et un objet "étendu" appartiennent à la même classe?
Ne serait-il pas juste de décrire les différents états possibles d'un objet en tant que classes ou interfaces distinctes?
Donc, mon idée était la suivante:
public Product fill(<? extends BaseProduct> product, Images images, Prices prices, Availabilities availabilities){ var p1 = priceFiller.fill(product, prices); var p2 = availabilityFiller.fill(p1, availabilities); return imageFiller.fill(p2, images); } PriceFiller public ProductWithPrices fill(<? extends BaseProduct> product, Prices prices) AvailabilityFiller public ProductWithAvailabilities fill(<? extends ProductWithPrices> product, Prices prices) public <BaseProduct & PriceAware & AvailabilityAware> fill(<? extends BaseProduct & PriceAware> product, Prices prices)
C'est-à-dire le produit initialement défini est une instance d'une classe autre que celle renvoyée, qui montre déjà les changements de données.
Filler , dans leurs API, spécifient exactement les données dont ils ont besoin et ce qu'ils renvoient.
De cette façon, vous pouvez empêcher la mauvaise séquence d'appels.
Implémentation
Comment traduire cela en réalité en Java? (Rappelons que l'héritage de plusieurs classes en Java n'est pas possible.)
La complexité est ajoutée par des opérations indépendantes. Par exemple, des images peuvent être ajoutées avant et après l'ajout de prix, ainsi qu'à la toute fin de la fonction.
Alors peut-être
class ProductWithImages extends BaseProduct implements ImageAware{} class ProductWithImagesAndPrices extends BaseProduct implements ImageAware, PriceAware{} class Product extends BaseProduct implements ImageAware, PriceAware, AvailabilityAware{}
Comment tout décrire?
Créer des adaptateurs?
public ProductWithImagesAndPrices(<? extends BaseProduct & PriceAware> base){ this.base = base; this.images = Collections.emptyList(); } public long getId(){ return this.base.getId(); } public Price getPrice(){ return this.base.getPrice(); } public List<Image> getImages(){ return this.images; }
Copier des données / liens?
public ProductWithImagesAndPrices(<? extends BaseProduct & PriceAware> base){ this.id = base.getId(); this.prices = base.getPrices(); this.images = Collections.emptyList(); } public List<Image> getImages(){ return this.images; }
Comme déjà visible, tout se résume à une énorme quantité de code. Et cela malgré le fait que dans l'exemple je n'ai laissé que 3 types de données d'entrée. Dans le monde réel, il peut y en avoir beaucoup plus.
Il s'avère que les coûts d'écriture et de maintenance d'un tel code ne se justifient pas, bien que l'idée de diviser l'État en classes séparées me paraisse très intéressante.
Retraite
Si vous regardez d'autres langues, alors quelque part ce problème est plus facile à résoudre, mais pas quelque part.
Par exemple, dans Go, vous pouvez écrire une référence à une classe extensible sans méthodes de «copie» ou de «surcharge». Mais il ne s'agit pas de Go
Une autre digression
Lors de l'écriture de cet article, une autre solution possible a été trouvée avec Proxy , qui ne demandait que l'écriture de nouvelles méthodes, mais nécessitant une hiérarchie d'interfaces. En général, effrayant, en colère et ne convient pas. Si quelqu'un est soudain intéressé:
Avant d'aller au lit et de manger, il n'est pas recommandé de regarder public class Application { public static void main(String[] args) { var baseProduct = new BaseProductProxy().create(new BaseProductImpl(100L)); var productWithPrices = fillPrices(baseProduct, BigDecimal.TEN); var productWithAvailabilities = fillAvailabilities(productWithPrices, "available"); var productWithImages = fillImages(productWithAvailabilities, List.of("url1, url2")); var product = productWithImages; System.out.println(product.getId()); System.out.println(product.getPrice()); System.out.println(product.getAvailability()); System.out.println(product.getImages()); } static <T extends BaseProduct> ImageAware fillImages(T base, List<String> images) { return (ImageAware) Proxy.newProxyInstance(base.getClass().getClassLoader(), new Class[]{ImageAware.class, BaseProduct.class}, new MyInvocationHandler<>(base, new ImageAware() { @Override public List<String> getImages() { return images; } })); } static <T extends BaseProduct> PriceAware fillPrices(T base, BigDecimal price) { return (PriceAware) Proxy.newProxyInstance(base.getClass().getClassLoader(), new Class[]{PriceAware.class}, new MyInvocationHandler<>(base, new PriceAware() { @Override public BigDecimal getPrice() { return price; } })); } static AvailabilityAware fillAvailabilities(PriceAware base, String availability) { return (AvailabilityAware) Proxy.newProxyInstance(base.getClass().getClassLoader(), new Class[]{AvailabilityAware.class}, new MyInvocationHandler<>(base, new AvailabilityAware() { @Override public String getAvailability() { return base.getPrice().intValue() > 0 ? availability : "sold out"; } })); } static class BaseProductImpl implements BaseProduct { private final long id; BaseProductImpl(long id) { this.id = id; } @Override public long getId() { return id; } } static class BaseProductProxy { BaseProduct create(BaseProduct base) { return (BaseProduct) Proxy.newProxyInstance(this.getClass().getClassLoader(), new Class[]{BaseProduct.class}, new MyInvocationHandler<>(base, base)); } } public interface BaseProduct { default long getId() { return -1L; } } public interface PriceAware extends BaseProduct { default BigDecimal getPrice() { return BigDecimal.ZERO; } } public interface AvailabilityAware extends PriceAware { default String getAvailability() { return "sold out"; } } public interface ImageAware extends AvailabilityAware { default List<String> getImages() { return Collections.emptyList(); } } static class MyInvocationHandler<T extends BaseProduct, U extends BaseProduct> implements InvocationHandler { private final U additional; private final T base; MyInvocationHandler(T base, U additional) { this.additional = additional; this.base = base; } @Override public Object invoke(Object proxy, Method method, Object[] args) throws Throwable { if (Arrays.stream(additional.getClass().getInterfaces()).anyMatch(i -> i == method.getDeclaringClass())) { return method.invoke(additional, args); } var baseMethod = Arrays.stream(base.getClass().getMethods()).filter(m -> m.getName().equals(method.getName())).findFirst(); if (baseMethod.isPresent()) { return baseMethod.get().invoke(base, args); } throw new NoSuchMethodException(method.getName()); } } }
Conclusion
Qu'est-ce qui se passe? D'une part, il existe une approche intéressante qui applique des classes distinctes à un "objet" dans différents états et garantit la prévention des erreurs causées par une séquence incorrecte d'appels aux méthodes qui modifient cet objet.
D'un autre côté, cette approche vous fait écrire tellement de code que vous voulez immédiatement le refuser. Un tas d'interfaces et de classes ne fait que compliquer la compréhension du projet.
Dans mon autre projet, j'ai quand même essayé d'utiliser cette approche. Et au début, au niveau de l'interface, tout allait bien. J'ai écrit les fonctions:
<T extends Foo> List<T> firstStep(List<T> ts){} <T extends Foo & Bar> List<T> nStep(List<T> ts){} <T extends Foo> List<T> finalStep(List<T> ts){}
Ayant ainsi indiqué qu'une certaine étape du traitement des données nécessite des informations supplémentaires qui ne sont nécessaires ni au début du traitement, ni à sa fin.
En utilisant mock 'et, j'ai réussi à tester le code. Mais en ce qui concerne la mise en œuvre et la quantité de données et de diverses sources a commencé à augmenter, j'ai rapidement abandonné et tout refait dans un aspect «normal». Tout fonctionne et personne ne se plaint. Il s'avère que l'efficacité et la simplicité du code l'emportent sur la "prévention" des erreurs, et vous pouvez tracer manuellement la bonne séquence d'appels, même si l'erreur ne se manifeste qu'au stade du test manuel.
Peut-être que si je prenais du recul et regardais le code de l'autre côté, j'aurais des solutions complètement différentes. Mais il se trouve que je me suis intéressé à cette ligne particulière commentée.
Déjà à la fin de l'article, en pensant au fait que comme il n'est pas agréable de décrire les setters dans les interfaces, vous pouvez imaginer l'assemblage des données produit sous la forme d'un Builder , qui renvoie une interface différente après l'ajout des données définies. Encore une fois, tout dépend de la complexité de la logique de construction des objets. Si vous avez travaillé avec Spring Security, vous connaissez ce type de solution.
Pour mon exemple, ça va:
Solution basée sur un modèle de générateur public class Application_2 { public static void main(String[] args) { var product = new Product.Builder() .id(1000) .price(20) .availability("available") .images(List.of("url1, url2")) .build(); System.out.println(product.getId()); System.out.println(product.getAvailability()); System.out.println(product.getPrice()); System.out.println(product.getImages()); } static class Product { private final int price; private final long id; private final String availability; private final List<String> images; private Product(int price, long id, String availability, List<String> images) { this.price = price; this.id = id; this.availability = availability; this.images = images; } public int getPrice() { return price; } public long getId() { return id; } public String getAvailability() { return availability; } public List<String> getImages() { return images; } public static class Builder implements ProductBuilder, ProductWithPriceBuilder { private int price; private long id; private String availability; private List<String> images; @Override public ProductBuilder id(long id) { this.id = id; return this; } @Override public ProductWithPriceBuilder price(int price) { this.price = price; return this; } @Override public ProductBuilder availability(String availability) { this.availability = availability; return this; } @Override public ProductBuilder images(List<String> images) { this.images = images; return this; } public Product build(){ var av = price > 0 && availability != null ? availability : "sold out"; return new Product(price, id, av, images); } } public interface ProductBuilder { ProductBuilder id(long id); ProductBuilder images(List<String> images); ProductWithPriceBuilder price(int price); Product build(); } public interface ProductWithPriceBuilder{ ProductBuilder availability(String availability); } } }
Alors que:
- Écrire des fonctionnalités propres
- Écrivez un code beau et clair
- N'oubliez pas que la brièveté est une sœur et surtout que le code fonctionne
- N'hésitez pas à remettre en question les choses, à chercher d'autres moyens et même à proposer des solutions alternatives, d'ailleurs
- Ne te tais pas! Lors de l'explication d'un problème aux autres, de meilleures solutions naissent (Rubber Duck Driven Developent)
Merci de votre attention.