Better way to do multiple if statements

I don't know if I worded that right but I'm trying to make a plugin and you can't eat meat so I made an if statement but its way too long.

Code:

if (player.getInventory().getItemInMainHand().getType().equals(Material.BEEF) || player.getInventory().getItemInMainHand().getType().equals(Material.COOKED_BEEF) || player.getInventory().getItemInMainHand().getType().equals(Material.CHICKEN) || player.getInventory().getItemInMainHand().getType().equals(Material.COOKED_CHICKEN) || player.getInventory().getItemInMainHand().getType().equals(Material.MUTTON) || player.getInventory().getItemInMainHand().getType().equals(Material.COOKED_MUTTON) || player.getInventory().getItemInMainHand().getType().equals(Material.PORKCHOP) || player.getInventory().getItemInMainHand().getType().equals(Material.COOKED_PORKCHOP) || player.getInventory().getItemInMainHand().getType().equals(Material.RABBIT) || player.getInventory().getItemInMainHand().getType().equals(Material.COOKED_RABBIT) || player.getInventory().getItemInMainHand().getType().equals(Material.COD) || player.getInventory().getItemInMainHand().getType().equals(Material.COOKED_COD) || player.getInventory().getItemInMainHand().getType().equals(Material.SALMON) || player.getInventory().getItemInMainHand().getType().equals(Material.COOKED_SALMON) || player.getInventory().getItemInMainHand().getType().equals(Material.TROPICAL_FISH) || player.getInventory().getItemInMainHand().getType().equals(Material.ROTTEN_FLESH))

It's this but copied for every meat in the game, and I can't figure out any other way to do this, plus I think It's crashing my server.

 

@Jcg0525 You could add the meat materials into a collection (e.g. a HashSet), then check if the collection contains the material in the player's hand. That would reduce the if-statement to the following:

Code:Java

Set<Material> meat = new HashSet<>();
// Add meat materials to the collection during initialization
 
Material heldMaterial = player.getInventory().getItemInMainHand().getType();
if (meat.contains(heldMaterial)) {
    // The player is holding meat
}

 

KarimAKL's suggestion is definitely the way to go. Just wanted to say if, for some weird reason, you had to do checks of that nature, you could clean that up and make it a lot more readable like:

Code:

Material heldType = heldType;
if (heldType != Material.BEEF &&
    heldType != Material.COOKED_BEEF &&
    heldType != Material.CHICKEN &&
    heldType != Material.COOKED_CHICKEN &&
    heldType != Material.MUTTON &&
    heldType != Material.COOKED_MUTTON &&
    heldType != Material.PORKCHOP &&
    heldType != Material.COOKED_PORKCHOP &&
    heldType != Material.RABBIT &&
    heldType != Material.COOKED_RABBIT &&
    heldType != Material.COD &&
    heldType != Material.COOKED_COD &&
    heldType != Material.SALMON &&
    heldType != Material.COOKED_SALMON &&
    heldType != Material.TROPICAL_FISH &&
    heldType != Material.ROTTEN_FLESH) return;

Bear in mind, I would assuredly NOT do this, lol. Just wanted to show that you could still make that mess a little more friendly to read. You can set a Material variable so you don't need to constantly be using long compound statements like you are. Also for an enum, you typically use == not equals. I also prefer to check for a negative condition and return rather than a positive one; prevents unnecessary indentation.

 

I just tried using this in my code and it did nothing, here's my code:

Code:

    @org.bukkit.event.EventHandler
    public void eatMeat(PlayerInteractEvent event) {
        Player player = event.getPlayer();

        Set<Material> meat = new HashSet<>();

        Material heldMaterial = player.getInventory().getItemInMainHand().getType();
        if (meat.contains(heldMaterial)) {
            if (event.getAction().equals(Action.RIGHT_CLICK_AIR) || event.getAction().equals(Action.RIGHT_CLICK_BLOCK)) {

                player.sendTitle("You're vegetarian", "You should be ashamed", 15, 35, 15);

                ItemStack noMeat = new ItemStack(Material.AIR);
                player.getEquipment().setItemInMainHand(noMeat);

                player.addPotionEffect(new PotionEffect(PotionEffectType.BLINDNESS, 125, 5));
                player.addPotionEffect(new PotionEffect(PotionEffectType.SLOW, 125, 5));
                player.addPotionEffect(new PotionEffect(PotionEffectType.HUNGER, 250, 5));
            }
        }
    }

Edit: am I supposed to add the materials or something? I'm confused sorry.

 

That's correct. You're supposed to add the materials after initializing the collection and before checking its contents. You can do this by using the Set#add(E) method.
Also, rather than creating the collection inside of the method, you can make it an instance variable by declaring it as a class member, then you can initialize it and add the materials in the class constructor.

 

Also you don't really need to make a variable for the air. Just set the item in main hand to null:

Code:

player.getEquipment().setItemInMainHand(null);

That will accomplish the same thing. If you were actually setting an item, like giving them 5 diamonds, you also could compact it by just doing:

Code:

player.getEquipment().setItemInMainHand(new ItemStack(Material.DIAMOND, 5));

Now if you intend to reference something multiple times, then yea, make a distinct variable. But for one offs like this, I personally prefer doing it inline. I like things compact, heh.