0

I am making a replica of a Subway restaurant where you would receive an order in a certain sequence and check if the sequence is valid and if the ingredients are in the menu.

The right order is: 1 bread, 0 to 1 meat, 1 cheese, 1 to 3 extras, 1 to 3 sauces.

Meaning that an order can have a minimum of 4 ingredients (bread, cheese, 1 extra, 1 sauce) and a maximum of 9 ingredients (bread, meat, cheese, 3 extras, 3 sauces).

My question is if there is a more optimized/smarter method to go about validating each and every ingredient than mine?

Code:

// Example order
HashMap<String, HashSet<String>> menu = new HashMap<>();

public static void main(String[] args) {
    // Example order
    List<String> ingredients = Arrays.asList("Wheat", "Veal",
        "Yellow", "Cucumbers", "Onions");
    if (!isValid(ingredients)) {
        // throw exc;
}
    
    
    
boolean isValid(List<String> ingredients) {
    if (ingredients.size() < 4 || ingredients.size() > 9) {
        return false;
    }
    int i = 0;
    // Bread
    if (!Restaurant.menu.get("Bread")
            .contains(ingredients.get(i++))) {
        System.out.println("Bread");
        return false;
    }
    
    // Meat
    if (!(Restaurant.menu.get("Meat")
            .contains(ingredients.get(i)))
            && !Restaurant.menu.get("Cheese")
                    .contains(ingredients.get(i))) {
        System.out.println("Meat");
        return false;
    }
    
    if (Restaurant.menu.get("Meat")
            .contains(ingredients.get(i))) { // Meat case
        if ((!Restaurant.menu.get("Cheese")
                .contains(ingredients.get(++i)))) {
            System.out.println("Cheese");
            return false;
        }
    }
    
    for (int j = ++i; j < ingredients.size(); j++) {
        if ((!Restaurant.menu.get("Extras")
                .contains(ingredients.get(j)))) { // Extras
            if (j == i) {
                return false;
            } else {
                if ((!Restaurant.menu.get("Sauces")
                        .contains(ingredients.get(j)))) { // Sauces
                    return false;
                }
            }
        }
    }
    
    return true;
}

Note 1: I know about the rule "If it works, don't touch it" but I feel like this code is getting in the territory of spaghetti code with a bunch of ifs essentially checking similar things with lots of cases and just wanted a second opinion if there is a more optimized way I can't think of right now.

Note 2: I chose HashSet over ArrayList for the menu because it is faster to search.

WJS
  • 36,363
  • 4
  • 24
  • 39
Deutrys
  • 15
  • 5
  • Why is the order of the ingredients important? Seems like just checking for the required ones in the right quantity is what is important. – WJS Jun 19 '22 at 13:37
  • 1
    You'll get more potential optimization (in the logical sense too) if you modelled your domain with different types. Strings don't make for a proper solution. That means different classes or enums – g00se Jun 19 '22 at 14:02
  • @WJS I want to imitate a real Subway restaurant. So when you go in you tell the ingredients in order so the employees can make it for you. You cant just make them remember 20 stuff and say the type of bread last for example – Deutrys Jun 19 '22 at 14:38
  • @g00se I was thinking about that, but thought it wouldn't be very well structured. How would I put "Meat" and the types of meat for example? 2 different enum classes or one with Meat, Ham, Turkey, etc..? – Deutrys Jun 19 '22 at 16:07
  • `enum Meat { Ham, Turkey, Beef }; //yum` ? Then you could have an `EnumSet`. But of course, you could go finer grained into a 'recipe' type of thing with measure ingredients – g00se Jun 19 '22 at 17:01

2 Answers2

0

The problem I see with your proposed solution is that it is trying to solve all problems at once, instead of solving them separately. That makes your code hard to read and understand, in my opinion. While it may work, the more business rules you add, the harder this will get.

So what can you do about it? Separate the concerns.

The first concern is cassifying an ingredient: is it bread, cheese, meat, extra, sauce? You could e.g. create a class Menu with a method getCategory() (instead of just using a HashSet for menu) that returns the category, and the return value could be an Enum.

The seond concern is order. You could check the order of the list using a custom Comparator. See this question for details.

The third concern is number of ingredients of a certain category. Given that you can find out the category of an ingredient, you can count how many you have and check if it is the right amount.

There are more things to be said about how to achieve any of this, I just wanted to point you in a possible direction.

bertilmuth
  • 276
  • 1
  • 11
0

First, there is nothing really wrong with your general approach.

Having said that, there are some errors.

  • You are referencing your hashMap as a static value via Restaurant where it isn't declared static.
  • you are calling isValid() from within a static context (Main) but the method is not declared static.

This is how I might approach it. And it's not to say it is the best approach or style either.

  • I chose to use an enum to hold details about each menu item, and a class to process the order. Enum's easily lend themselves to this type of requirement. I recommend you read up on them (they are too involved to explain them in detail here). But because they are static they are best served in holding values that are not likely to change.

  • Each item has two arguments are are processed in the enum's constructor.

  • so each item has a separate range of its item (the ingredient)

  • the enum also has a validate method to check to see if a supplied count of items meets the requirements.

The actual order is in the MyOrder class.

  • it contains a map to hold the counts of each ingredient.
  • an add method to add the current quantity to the map for a given ingredient.
  • and a display method to print informative information about the order.
enum Menu {MEAT(0,1), BREAD(1,1), CHEESE(1,1), EXTRAS(1,3), SAUCES(1,3);
    private int min;
    private int max;
    private Menu(int min, int max) {
        this.min = min;
        this.max = max;
    }
    
    public int getMax() {
        return max;
    }
    public int getMin() {
        return min;
    }
    
    public  boolean validate(int count) {
        return count >= min && count <= max;
    }
}
class MyOrder {
    private EnumMap<Menu, Integer> counts = new EnumMap<>(Menu.class);
    
    public void display() {
        for (Menu item : Menu.values()) {
            int count = counts.get(item);
        System.out.printf("%-7s: %d  (%d,%d) %s%n",item.name(), count, item.getMin(), item.getMax(),
                item.validate(count) ? "" : "Item count out of range.");
        }
    }
        
    public boolean add(Menu item, int quantity) {
        return item.validate(counts.merge(item, quantity, Integer::sum));
    }
}
        
public class Restaurant {

    public static void main(String[] args) {
        boolean isValid;
        MyOrder order = new MyOrder();
        isValid =  order.add(Menu.BREAD,2);
        isValid &= order.add(Menu.MEAT,1);
        isValid &= order.add(Menu.CHEESE,2);
        isValid &= order.add(Menu.EXTRAS,3);
        isValid &= order.add(Menu.SAUCES,2);
        if (isValid) {
            System.out.println("Your order is accepted.");
        } else {
            System.out.println("Order is not in compliance");
            order.display();
        }
    }
}

prints

Order is not in compliance
MEAT   : 1  (0,1) 
BREAD  : 1  (1,1) 
CHEESE : 2  (1,1) Item count out of range.
EXTRAS : 3  (1,3) 
SAUCES : 2  (1,3)

Also remember that the result of any if statement is a boolean. So the inequality can be assigned to a boolean and then tested later (if it makes sense to do so). Also notice that I don't check for a legitimate order until the end. Some might prefer to signal a bad order as soon as it occurs. This is a design choice.

For more information check.
Enum
EnumMap
Map.merge

WJS
  • 36,363
  • 4
  • 24
  • 39