Solved Need advice regarding a particular piece of code

Discussion in 'Spigot Plugin Development' started by Diamond_X, Dec 30, 2018.

  1. Hey,

    Recently started coding again, and created a private plugin for myself but want to clean up the code a bit now. The main thing I'm looking for is shortening the code.

    Currently I'm looking what the player has in his main hand. And if it's one of the items I want it to have, I want it to execute a thing.

    Now I couldn't figure out how to have all the items I want without copy pasting the same thing over and over again with a different material name.

    For example, I want it to execute with this this this and this item, but not with any other item. If that makes sense.

    Code (Java):
    if (player.getInventory().getItemInMainHand().getType() == Material.WOODEN_HOE) {
    Could anyone suggest something?
     
  2. You could store the materials in an unmodifiable list and check and see if it contains the material instead of a bunch of if statements, however, this would only work if you want the same code to run for all the materials in the list.
     
  3. It is basically all the same thing that gets executed, only the material is different, haha.
     
  4. What code do you keep repeating that you want to shorten?
     
  5. Also check if item in hand isnt null!
     
  6. Could you not just do something like

    Code (Java):
    Object mainItem = player.getInventory().getItemInMainHand().getType()
    obviously don't use the Object class but rather replace it with whatever type the getType method returns

    Then just test what mainItem is?

    That's assuming you mean that in a single method you are repeating the code alot
     
  7. Something along these lines is what I would do...

    First, static import the Material class
    Code (Java):
    import static org.bukkit.Material.*;
    Create an array of Materials you want to check against
    Code (Java):
    private final Material[] materialArray = {WOODEN_HOE, OTHER_MATERIAL};
    Check against it like so...
    Code (Java):
    if (Arrays.stream(materialArray).anyMatch(m -> m == held)) {
        //The item held is the same as one of the items in the array
    }
     
    • Agree Agree x 1
  8. Use a switch statement. Unless you break for whichever case, it falls through to whatever the next case does.
    Example:
    Code (Java):
    switch (item.getType()) {
        case WOOD:
        case STONE:
            // code..
            break;
        case DIRT:
        case GRASS:
            // code..
            break;
    }
    As for stuffing a bunch of types into a single statement because you're getting an object each time, just set variables and use them. Instead of player.getInventory() 10 times, Inventory inv = player.getInventory() once and suddenly your code in that method or whatever from now on is that much cleaner.
     
    #8 Escad, Dec 30, 2018
    Last edited: Dec 30, 2018
    • Agree Agree x 1
  9. That's alot cleaner then my method, ONLY if the OP wants to do something different dependant on the material, my method is much cleaner though if the OP just wants to check against a certain set of materials and do the same thing, which is what OP mentioned they wanted
     
  10. If it's a never-changing set of materials that they intend to use, a switch statement would be better, as there's no need to create and add to a collection/array. Using streams unnecessarily is slow.
     
  11. Just so I get it right and understand it, something like this?

    Code (Java):
    case WOODEN_SWORD:
    case WOODEN_PICKAXE:
    case WOODEN_AXE:
    case etc etc..
             Run what needs to be executed here
    And it would work for all the cases or do I need to run what needs to be run under each case like.


    Code (Java):
    case WOODEN_SWORD:
             Execute this
    case WOODEN_PICKAXE:
             Execute this
     
  12. Yup, right with the first one. You can just stick together all of the cases and they'll execute any code in the switch statement until it reaches a break statement.
     
  13. Choco

    Junior Mod

    For anyone reading this in the future, opt for an EnumSet instead. It will be far more performant to use its #contains() method rather than creating a Stream and iterating as an ArrayList would do.
     
  14. Oh, thank you. I read up on switch and case but never thought of it this time around neither have I used it.

    Could this also be used like this?

    Code (Java):
    case WOODEN_PICKAXE:
             if (player.hasPermission("wooden.pick")) {
                 execute something
             } else {
                 Denying it
    And if it does, would it then just skip to the next case if the person does not have permission?
     
  15. Fair fair, performance could be an issue, but we're talking less than a few millseconds here. EnumSet is definitely the best choice in this case though, completely forgotten about that ;')
     
  16. switch case is equivalent to a bunch of if else statements, just a lot cleaner, so yes it can do that.
    However, use an EnumSet as @2008Choco suggested and use it's #contains method, otherwise you're defeating the purpose of the DRY principle with a switch case statement in the use case you described
    So...
    Code (Java):
    private final EnumSet<Material> set = EnumSet.of(WOODEN_HOE, OTHER_MATERIAL)
    Then check with
    Code (Java):
    set.contains(material);
     
  17. A case like that is fine, assuming that you add a break statement if the person does have permission, but generally it's more useful when you intend for different items to result in common functionality via switch and case.
    Choco's been around here much longer and has more Java experience than I do. I haven't used EnumSets, so I couldn't tell you the difference besides switch and case looks a bit messier with more values.
     
  18. Choco

    Junior Mod

    At this point its a matter of preference. You could argue that a switch or an EnumSet would be equivalent with the exception that you'd be using switch cases vs. if/else-if. Although,
    Follow the Liskov Substituiton Principle and declare the type as a set.
    Code (Java):
    private static final Set<Material> HOES = EnumSet.of(Material.WOODEN_HOE, Material.STONE_HOE /*, etc. */);
     
    • Like Like x 2
  19. Out of interest, is the second option just to keep things more organised or does it affect performance in anyway?
     
  20. Choco

    Junior Mod

    Between an EnumSet and a switch, the only two possible differences I can imagine are code readability (preference) and the fact that you're using a collection vs. constants. You could argue that the use of a collection is going to populate the stack, but really... one EnumSet is going to make a negligible difference. Whichever one you prefer. Would you rather use a switch or if/else? That's your answer :p
     

Share This Page