Conception d'une structure de code propre orientée objet avec les services Spring
J'écris une application de printemps pour stocker et manipuler des recettes. J'ai écrit le service ShoppingList qui génère, sur la base des UUID des recettes, des listes de courses. C'est mon modèle actuel pour lequel j'ai écrit des tests et fonctionne comme prévu.
Le service utilise un autre service appelé IngredientConverterqui se convertit en modèles de réponse. Maintenant, je voulais déplacer une certaine dépendance ShoppingListcar cette fonction est jalouse des fonctions, mais aborde un problème. Comment une classe peut-elle utiliser le IngredientConverterservice sans le passer laid pour ShoppingListfonctionner? Y a-t-il des conseils que vous pourriez donner à mon code et à la planification de ma structure?
@Service
@AllArgsConstructor
public class ShoppingListService {
private final RecipeService recipeService;
private final IngredientConverter ingredientConverter;
public ShoppingList generateShoppingList(List<UUID> uuidsOfRecipes) {
List<Recipe> recipes = recipeService.getAllByIDIn(uuidsOfRecipes);
ShoppingList shoppingList = ShoppingList.empty();
Map<Ingredient, Integer> ingredients = new HashMap<>();
recipes.forEach(recipe ->
recipe.getIngredients().forEach(
ingredientQuantity ->
ingredients.compute(
ingredientQuantity.getIngredient(),
(key, value) ->
value == null ?
ingredientQuantity.getAmount() :
value + ingredientQuantity.getAmount())));
ingredients.keySet().forEach(
ingredient ->
shoppingList.getIngredients().add(
ingredientConverter.convertWithAmount(
ingredient.getName(),
ingredients.get(ingredient),
ingredient.getUnit())
)
);
return shoppingList;
}
}
@Data
@AllArgsConstructor
public class ShoppingList {
private final List<IngredientQuantity> ingredients;
public static ShoppingList of(List<IngredientQuantity> ingredients) {
return new ShoppingList(ingredients);
}
public static ShoppingList empty(){
return ShoppingList.of(new ArrayList<>());
}
}
@Service
public class IngredientConverter {
public IngredientQuantity convertWithAmount(String name, int amount, Unit unit) {
return IngredientQuantity.builder()
.amount(amount)
.ingredient(convert(name, unit))
.build();
}
}
@Data
@Entity
@SuperBuilder
@NoArgsConstructor
@AllArgsConstructor
@EqualsAndHashCode(callSuper = true)
public class IngredientQuantity extends IdentifiableEntity {
private int amount;
@ManyToOne
private Ingredient ingredient;
}
@Data
@Getter
@Entity
@SuperBuilder
@NoArgsConstructor
@AllArgsConstructor
@EqualsAndHashCode(callSuper = false)
public class Ingredient extends IdentifiableEntity {
private String name;
@Enumerated(EnumType.STRING)
private Unit unit;
}
@Getter
@NoArgsConstructor
@SuperBuilder
@MappedSuperclass
public class IdentifiableEntity {
@Id
@GeneratedValue(strategy = GenerationType.AUTO)
private UUID ID;
}
```
Réponses
J'injecterais le converterservice dans ShoppingList et implémenterais une fonction add (), qui ajoute un élément à la ShoppingList après la conversion.
void add(name, ingredient, unit) {
IngredientQuantity toAdd = this.converterService.convert(name, ingredient, unit); // terrible naming, todo!
this.ingredients.add(toAdd);
}
Maintenant, la liste de courses est le propriétaire de la conversion qui (sans doute) modélise la vie réelle. Quand j'ai besoin d'acheter du lait, je convertis les recettes américaines en litres au lieu d'onces ou autre.
Cela simplifie également votre dernière fonction lambda, il vous suffit de parcourir le jeu de clés et d'appeler ShoppingList :: add, qui est la syntaxe de référence de la méthode Java8 +.
L'autre énorme lambda devrait probablement aussi être divisé en parties, je veux dire, j'aime le java fonctionnel autant que quiconque, mais je crois que cela coupe la ligne de l'utilité fonctionnelle sur la lisibilité. Pseudocode
foreach Recipe:
foreach Ingredient : Recipe
compute(ingredient)
Ce sont mes 2 cents.
Ruisseaux
Quelques points à garder à l'esprit.
La nouvelle syntaxe de .forEach () est cool mais elle devrait être utilisée pour des choses simples et elle ne devrait pas être imbriquée. Refactorisez-le pour éviter l'imbrication ou extrayez les parties imbriquées dans des méthodes. De cette façon, ces parties auront des noms qui devraient rendre leur but évident.
Ne modifiez pas les collections avec .forEach. Si vous avez besoin de traiter une collection, utilisez des flux et des collecteurs pour créer la collection que vous désirez. Cela éliminera le couplage temporel entre la création de collections vides et leur remplissage.
public ShoppingList generateShoppingList(List<UUID> uuidsOfRecipes) { List<Recipe> recipes = recipeService.getAllByIDIn(uuidsOfRecipes); Map<Ingredient, Integer> ingredients = recipes.stream() .flatMap(recipe -> recipe.getIngredients().stream()) .collect(Collectors.toMap( IngredientQuantity::getIngredient, IngredientQuantity::getAmount, Integer::sum)); # Next step would be to refactor this piece. ShoppingList shoppingList = ShoppingList.empty(); ingredients.forEach((ingredient, amount) -> shoppingList.getIngredients().add( ingredientConverter.convertWithAmount( ingredient.getName(), amount, ingredient.getUnit()) )); return shoppingList; }
Remarque: le code n'a pas été testé.
Général
- Je suggérerais de supprimer le fichier
IngredientConverter. Cela ne sert à rien car in peut être remplacé par un constructeur. - Le seul objet avec une logique significative est
ShoppingListService. Vous pouvez rendre votre code plus orienté objet en déléguant une partie de la logique à d'autres parties. Par exemple, la liste d'achats peut être responsable de l'agrégation des ingrédients. Appliquez-add(IngredientQuantity)le simplement et laissez-le gérer les totaux d'ingrédients. Cela simplifierait davantage le service car il n'aura pas à convertir en cartes et à calculer le total là-bas.