I'm currently working through a tutorial. The project is a recipe website. One of the lessons to "save some lines of code" and use bidirectional setters for bidirectional relations of classes/objects. Namely Recipe <-> Notes.
In the tutorial, the guy simply uses the Recipe.setNotes(Notes)
setter to create that relationship. As suggested by the method name, notes is set for recipe. But also, and this is not obvious, recipe is set for notes (Notes.setRecipe(Recipe)
).
I'm not sure if I like the name of a function like that. Sure, it's not unusual behaviour for a setter to have some sideeffects within the code. Things like validation come to mind. Or maybe it influences another property of that object. However, to pass an object into a funciton and unexpectedly have that function alter the object doesn't feel right.
Should this even be done?
I'm conflicted. On one hand I think it's a good solution, because it counteracts the mistake of setting the relationship one-directional. Meaning e.g. that I would only set Recipe.setNotes()
and forget to set Notes.setRecipe()
On the other hand, it's odd to have what is essentially a model object alter another model object. It seems to me like that shouldn't be it's responsibility.
Another alternative would be to have a whole nother class/object manage both of them and have a function joining both objects together by calling the respective setters. However, that feels like overkill.
What would be a good naming convention?
Assuming this is a valid step to take.
I thought of using the database lingo and instead of using setNotes
create a method called Recipe.join(Notes)
, Recipe.joinWith(Notes)
or Recipe.joinNotes(Notes)
to indicate that the objects get interconnected.
Another idea was to use either of these verbs:
- (inter)connect
- link
- merge
Although link and merge might be a bit misleading as they have different meanings in the sense of weblinks and git merges. And interconnected might be a mouth full (things like Recipe.interconnectWithDescription(Description)
are a bit long for something thats essentially a setter)
Additional complications:
The whole thing gets even messier when I consider Ingredients and Recipes. Recipes
has a list of Ingredient
so its a OneToMany relationship. Usually the word that is used here would be "add", which implies that we are dealing with a collction. Meaning that names like Recipe.joinIngredient
would take away that implicit information and make it look like a OneToOne relationship.
Recipe class:
@Entity
@Getter
@Setter
public class Recipe {
@Id
@GeneratedValue(strategy = GenerationType.IDENTITY)
private Long id;
private String description;
private Integer prepTime;
private Integer cookTime;
private Integer servings;
private String source;
private String url;
private String directions;
// TODO add
// private Difficulty difficulty;
@OneToMany(cascade = CascadeType.ALL, mappedBy = "recipe")
private Set<Ingredient> ingredients = new HashSet<>();
@Lob
private Byte[] image;
@Enumerated(value = EnumType.STRING)
private Difficulty difficulty;
@OneToOne(cascade = CascadeType.ALL)
private Notes notes;
@ManyToMany
@JoinTable(name = "recipe_category",
joinColumns = @JoinColumn(name = "recipe_id"),
inverseJoinColumns = @JoinColumn(name = "category_id"))
private Set<Category> categories = new HashSet<>();
public void setNotes(Notes notes) {
this.notes = notes;
notes.setRecipe(this);
}
public Recipe addIngredient(Ingredient ingredient) {
ingredient.setRecipe(this);
this.ingredients.add(ingredient);
return this;
}
}
Notes class:
@Entity
@Getter
@Setter
@NoArgsConstructor
public class Notes {
@Id
@GeneratedValue(strategy = GenerationType.IDENTITY)
private Long id;
@OneToOne()
private Recipe recipe;
@Lob
private String recipeNotes;
}
Ingredient class:
@Entity
@Getter
@Setter
@NoArgsConstructor
public class Ingredient {
@Id
@GeneratedValue(strategy = GenerationType.IDENTITY)
private Long id;
private String description;
private BigDecimal amount;
@ManyToOne
private Recipe recipe;
@OneToOne(fetch = FetchType.EAGER)
private UnitOfMeasure unitOfMeasure;
public Ingredient(String description, BigDecimal amount, UnitOfMeasure unitOfMeasure) {
this.description = description;
this.amount = amount;
this.unitOfMeasure = unitOfMeasure;
}
}
Another class that creates and the objects calls them like this:
Recipe guacRecipe = new Recipe();
guacRecipe.setDescription("Perfect Guacamole");
guacRecipe.setPrepTime(10);
guacRecipe.setCookTime(0);
guacRecipe.setDifficulty(Difficulty.EASY);
guacRecipe.setDirections("Dummy Directions");
Notes guacNotes = new Notes();
guacNotes.setRecipeNotes("Dummy Recipe Notes");
guacRecipe.setNotes(guacNotes);
guacRecipe
.addIngredient(new Ingredient("ripe avocados", new BigDecimal(2), eachUom))
.addIngredient(new Ingredient("Kosher salt", new BigDecimal(".5"), teaspoonUom))
.addIngredient(new Ingredient("fresh lime juice or lemon juice", new BigDecimal(2), tablespoonUom))
.addIngredient(new Ingredient("minced red onion or thinly sliced green onion", new BigDecimal(2), tablespoonUom))
.addIngredient(new Ingredient("serrano chillies, stems and seeds removed, minced", new BigDecimal(2), eachUom))
.addIngredient(new Ingredient("Cilantro", new BigDecimal(2), tablespoonUom))
.addIngredient(new Ingredient("freshly grated black pepper", new BigDecimal(2), dashUom))
.addIngredient(new Ingredient("ripe tomato, seeds and pulp removed, chopped", new BigDecimal("0.5"), eachUom));
-
1\$\begingroup\$ I changed the title so that it describes what the code does per site goals: "State what your code does in your title, not your main concerns about it.". Please check that I haven't misrepresented your code, and correct it if I have. \$\endgroup\$Toby Speight– Toby Speight2023年01月25日 09:46:35 +00:00Commented Jan 25, 2023 at 9:46
-
\$\begingroup\$ @TobySpeight I adjusted the question, hope that helps. \$\endgroup\$Benjamin Basmaci– Benjamin Basmaci2023年01月25日 15:11:46 +00:00Commented Jan 25, 2023 at 15:11
-
\$\begingroup\$ Include your imports. Where does Entity come from? \$\endgroup\$Reinderien– Reinderien2023年01月25日 23:04:19 +00:00Commented Jan 25, 2023 at 23:04
1 Answer 1
You voiced some reservations, and I agree with them. Define a utility helper method that associates one with the other.
associate(notes, recipe);
Or call it link()
, if you like.
It seems slightly unusual that a Notes
would need
a link to its parent Recipe
.
Maybe having a one-way link would suffice? And the method calls which mention notes should instead be taking a recipe argument?
I get the sense that eventually you will have some interesting number of recipes, and they will be persisted in CSV files or in a relational database. Rather than focusing on java objects, you might be better served by initially focusing on a relational schema which includes FK foreign key and many-to-many ingredient relationships. And then your ORM or DB layer of java code will quite naturally fall out of what's happening in the RDBMS.
For example, it's not clear to me that it's really desirable for an ingredient like "avocado" to maintain a bunch of pointers to "guacamole", "toast", and so on. Surely it would be better to query your external datastore for relevant recipes each time a user says they have avocado on hand that they wish to use, right? That's what CREATE INDEX is for.
Switching gears, this seems inconvenient:
new Ingredient("Kosher salt", new BigDecimal(".5"), teaspoonUom))
Recommend you arrange for an alternate signature which would also accommodate
new Ingredient("Kosher salt", .5, teaspoonUom))
-
\$\begingroup\$ "assoc" why not "associate"? Other terms that come to mind are
setChildNotes
,adoptNotes
or, depending on context,replaceNotes
might be fitting. A slightly different approach would be to not allow setting ofNotes
at all, but only provide a an instance trough a getterRecipe.getNotes()
which can be changed. Though, I'm not sure how Springs Entity system will like that. \$\endgroup\$Bobby– Bobby2023年06月26日 11:22:16 +00:00Commented Jun 26, 2023 at 11:22