В разработке Плагин на выпадение спавнеров с определенным шансом

Всем привет, написал плагин на выпадение спавнеров игроку с определенным шансом. Задумка следующая: игрок может получит спавнер, если сломал его алмазной или незеритовой киркой с шелковым касанием. Также установлены разные шансы для игроков с разными привилегиями. Хотел бы получить что-то вроде код-ревью. Может недочеты по стилю кода или еще какие-нибудь ошибки есть.
Ссылка на код: https://pastebin.com/F65zQUHg

 

Подправил твой код, вот, что конкретно я сделал:

• Переименовал метод слушателя из BlockBreakedEvent в onBlockBreakEvent. Методы в Java всегда стоит назвать с маленькой буквы, это негласное правило. Помимо этого, на момент вызова события блок ещё не сломан - ты можешь отменить это действие. Поэтому использование Break в прошедшем времени не совсем корректно
  
  
• Убрал каст BlockBreakEvent.getPlayer() к Player. Этот метод уде возвращает игрока
  
  
• Вынес в отдельные переменные блок и предмет в руке игрока:
  
  PHP:

  Block block = event.getBlock();
ItemStack itemInMainHand = player.getInventory().getItemInMainHand();
  Строки стали гораздо короче, читабельность увеличилась, меньше дублирования кода
  
  
• Заменил
  
  PHP:

  itemInMainHand.getEnchantments().containsKey(Enchantment.SILK_TOUCH)
  на
  
  PHP:

  itemInMainHand.containsEnchantment(Enchantment.SILK_TOUCH)
• Вынес некоторые условия в отдельные булин-флаги: validItem и validEnchant, благодаря чему удалось реорганизовать условия с целью сделать весь метод более читабельными:
  
  PHP:

  if (validItem && validEnchant) {
    ...
} else {
    ...
}
  
  
• Избавился от лесенок условий: инвертировал условия и применил return, где условие не соблюдено:
  
  PHP:

  if (event.getBlock().getType() != Material.SPAWNER) return;
• Изменил систему проверки прав. Честно говоря, я вообще не понял, как работает текущая система. Есть 4 разных права:
  blockBreaked.shogun - дает шанс выпадения 30%
  blockBreaked.samurai - дает шанс выпадения 100%
  blockBreaked.tramp и blockBreaked.default - позволяют получать сообщения о нехватке прав
  
  Зачем для вывода сообщения о нехватке прав проверять ещё какие-то права? Мне показалось это странным, поэтому избавился от blockBreaked.tramp и blockBreaked.default вовсе
  
  
• Поменял местами проверку права и генерацию шанса, т.к. это более логично и проще читается:
  
  PHP:

  if ((player.hasPermission("blockBreaked.shogun") && (random.nextDouble() < chanceShogun))) {
• Избавился от дублирования кода выдачи предмета. Так и читабельней, и не будет проблем, если решишь добавить предмету какие-то новые параметры. Раньше можно было добавить в одном месте и забыть про второе. А если таких мест будет 10 - вероятность ошибки возрастает существенно.
  
  
• Чуть позже заметил ещё одну проблему в коде - если у игрока есть необходимые права, но не отработал шанс выпадения, то игрок всё равно получит ошибку "У вас должна быть как минимум привилегия...".
  Чтобы это исправить - пришлось ещё немного изменить логику условий. Благодаря этому ещё и появилась возможность сообщить игроку о неудачном шансе (не)выпадения

Конечный код выглядит так:

PHP:

@EventHandler
public void onBlockBreakEvent(BlockBreakEvent event) {
    Block block = event.getBlock();
    if (block.getType() != Material.SPAWNER) return;

    Player player = event.getPlayer();
    ItemStack itemInMainHand = player.getInventory().getItemInMainHand();

    boolean validItem = itemInMainHand.getType() == Material.DIAMOND_PICKAXE
        || itemInMainHand.getType() == Material.NETHERITE_PICKAXE;

    boolean validEnchant = itemInMainHand.containsEnchantment(Enchantment.SILK_TOUCH);

    if (!validItem || !validEnchant) {
        player.sendMessage("§c§lЧтобы получить спавнер, у вас должна быть алмазная или незеритовая кирка с шелковым касанием!");
        return;
    }

    boolean dropItem = false;
    if ((player.hasPermission("blockBreaked.shogun"))) {
        if ((random.nextDouble() < chanceShogun)) {
            dropItem = true;
        }
    } else if ((player.hasPermission("blockBreaked.samurai"))) {
        if ((random.nextDouble() < chanceSamurai)) {
            dropItem = true;
        }
    } else {
        player.sendMessage("§c§lУ вас должна быть как минимум привилегия §fꑴ §c§l для выпадения спавнеров!");
        return;
    }

    if (!dropItem) {
        player.sendMessage("§c§lНе повезло, блок не выпал :(");
        return;
    }

    ItemStack spawner = new ItemStack(block.getType(), 1);
    Location blockLocation = block.getLocation();
    block.getWorld().dropItemNaturally(blockLocation, spawner);
}

Помимо этого, я бы рекомендовал тебе добавить пользователям плагина возможность создавать свои собственные группы и шансы для них. Практически со 100% вероятностью такая потребность будет у пользователей твоего плагина. Заодно получишь опыт работы с Map

 

Спасибо большое) Вообще, это личный плагин, не планирую его в общий доступ закидывать, поэтому решил не делать возможность создавать группы

 

Кстати, а что такое Map?

 

Карта значений, в других языках часто называют словарём. Фактически это таблица ключ-знаение.
В твоём случае можно было бы сделать что-то типа ключ - название группы, а значение - шанс выпадения спаунера.
В интернете полно информации про Map и конкретно HashMap в Java, думаю разберёшься )

 

Понял, благодарю