HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMinor

Designing yet another coffee machine, Lombok style

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
designingcoffeeyetstyleanotherlombokmachine

Problem

After reading Designing a coffee machine, I decided to implement the same problem as an exercise to get to know Guava and Lombok.

I used the problem statement from the given question:

Design a coffee machine which makes different beverages based on set ingredients. The initialization of the recipes for each drink should be hard-coded, although it should be relatively easy to add new drinks. The machine should display the ingredient stock (+cost) and menu upon startup, and after every piece of valid user input. Drink cost is determined by the combination of ingredients. For example, Coffee is 3 units of coffee (75 cents per), 1 unit of sugar (25 cents per), 1 unit of cream (25 cents per). Ingredients and Menu items should be printed in alphabetical order. If the drink is out of stock, it should print accordingly. If the drink is in stock, it should print "Dispensing: ". To select a drink, the user should input a relevant number. If they submit "r" or "R" the ingredients should restock, and "q" or "Q" should quit. Blank lines should be ignored, and invalid input should print an invalid input message.

They supplied the default ingredients (&stock @10) and drinks/recipes.

In my version, I attempted to do the following:

  • Make everything immutable that is logical to be immutable



  • Separate the user-io from the data structures as much as possible, so that designing a different user interface becomes easy.



  • Make object creation as painless as possible through use of the builder pattern



I'm not sure if the builder pattern was the right way to go, however.
Main.java

```
package coffee;

import com.google.common.collect.Range;

import java.io.BufferedReader;
import java.io.IOException;
import java.io.InputStreamReader;
import java.util.List;
import java.util.SortedMap;

public class Main {
public static void main(String[] args) {
DrinkMachine drinkMachine = DrinkMachine.builder()
.drink(Recipe.builder().name("Coffee")

Solution

Money

I'm not exactly sure why you need a String-based valueOf() method here, when you are only using the other one that takes double values. Wouldn't it be simpler to just have your Money class accept cents as the base unit, and then set BigInteger value with BigInteger.valueOf()?

Receipe and the builder pattern

Lombok's builder pattern removes boilerplate code at the (slight) expense of flexibility in expressiveness. What I mean by that is that while it's nice for it to readily provide the name() and ingredient() methods, sometimes you may encounter other patterns that:

  • Use an external -Builder class, i.e. ReceipeBuilder in your case,



  • Use verbs to describe the action, i.e. addIngredient() instead of ingredient() (I understand this is just a simple change on your part),



  • Specify the name at the final build 'step', e.g. ReceipeBuilder.addIngredient(...).create("name").



My take on this is that there's really no issues with using Lombok's builder pattern, but at the same time don't be too limited by what you can do through a framework. :)

I think Receipe.validateIngredientList() can be made redundant if you were to work with an Set/ImmutableSet directly, instead of a List. Even if you will like to stick to this current approach, this method can be rewritten as such to leverage on the return value from Set.add, which is true only if the addition modifies the Set:

private void validateIngredientList(List ingredientList) {
    Set knownIngredients = new HashSet<>();
    for (IngredientListing listing : ingredientList) {
        checkArgument(knownIngredients.add(listing.getIngredient()),
                "Ingredient %s was declared multiple times in the recipe", 
                listing.getIngredient().getName());
    }
}


IngredientListing

You can consider using enum types for your various ingredients, if you are fine with defining the 'universe' of ingredients first.

Comparator

Since you are already using a healthy dose of Java 8 features, there's the newer Comparator.comparing() method that (arguably) makes these declarations more readable, e.g.

@Value
@EqualsAndHashCode(exclude = { "price" })
public class Ingredient {
    public static final Comparator NAME_COMPARATOR = 
                            Comparator.comparing(Ingredient::getName);
    public static final Comparator PRICE_COMPARATOR = 
                            Comparator.comparing(Ingredient::getPrice);
    private String name;
    private Money price;
}


Now, I must say this is totally untested as I don't have Lombok... maybe the method reference way of declaration might not work.

Main

  • Recommended to put the declaration of your drinks into its own method, e.g. private static DrinkMachine createDrinks().



  • Reading from the console can be done and validated in its own method, e.g. private static String getInput(Scanner).



-
Short of turning your collection of drinks into an enum type so that you can reference their ordinal() methods, here is another alternative implementation of displaying it that you may want to consider:

private static void printMenu(List drinks) {
    System.out.println("Menu:");
    int[] counter = new int[1];
    drinks.forEach(drink -> System.out.printf("%d: %s %s%n", ++counter[0],
                    drink.getPrice(), drink.getName());
    System.out.println("r: Restock");
    System.out.println("q: Quit");
    System.out.println();
}


The trick here is to rely on a single-element int[] array (effectively making a stream-friendly counting Object) as an index for the drink to be displayed.

Code Snippets

private void validateIngredientList(List<IngredientListing> ingredientList) {
    Set<Ingredient> knownIngredients = new HashSet<>();
    for (IngredientListing listing : ingredientList) {
        checkArgument(knownIngredients.add(listing.getIngredient()),
                "Ingredient %s was declared multiple times in the recipe", 
                listing.getIngredient().getName());
    }
}
@Value
@EqualsAndHashCode(exclude = { "price" })
public class Ingredient {
    public static final Comparator<Ingredient> NAME_COMPARATOR = 
                            Comparator.comparing(Ingredient::getName);
    public static final Comparator<Ingredient> PRICE_COMPARATOR = 
                            Comparator.comparing(Ingredient::getPrice);
    private String name;
    private Money price;
}
private static void printMenu(List<Recipe> drinks) {
    System.out.println("Menu:");
    int[] counter = new int[1];
    drinks.forEach(drink -> System.out.printf("%d: %s %s%n", ++counter[0],
                    drink.getPrice(), drink.getName());
    System.out.println("r: Restock");
    System.out.println("q: Quit");
    System.out.println();
}

Context

StackExchange Code Review Q#101226, answer score: 4

Revisions (0)

No revisions yet.