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.
CodePudding user response：
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.
CodePudding user response：
First, there is nothing really wrong with your general approach. Programming is a lot like golf. It's not about how, but how many. All folks see is the score card. In programming, results are important. Style comes with experience.
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
enumto 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 arestaticthey are best served in holding values that are not likely to change.Each item has two arguments are are processed in the
enum'sconstructor.so each item has a separate range of its item (the ingredient)
the
enumalso 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.