Bukkit/Spigot plugin for Minecraft servers

Question

I have created a Bukkit/Spigot plugin for Minecraft servers because I wanted to change the dynamic of mining to prevent people with really bright monitors or the Fullbright mod from being able to skip using torches when mining.

I created this plugin a while back when I used to run a Minecraft server, but since then have updated it a bit to allow a chance of not receiving broken blocks instead of just preventing them from being broken. It's fully configurable.

It functions fully besides a few data validation checks. You can skip those if you want since this site isn't about fixing invalid code.

This is my first open source project, so I would love any contributions or ideas!

The code is also on Github.

com/imcpwn/nofullbrightmining/NoFullBrightMining.java

/* NoFullBrightMining Bukkit/Spigot plugin by IMcPwn.
 * Copyright (C) 2016 IMcPwn

 * This program is free software: you can redistribute it and/or modify
 * it under the terms of the GNU General Public License as published by
 * the Free Software Foundation, either version 3 of the License, or
 * (at your option) any later version.

 * This program is distributed in the hope that it will be useful,
 * but WITHOUT ANY WARRANTY; without even the implied warranty of
 * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
 * GNU General Public License for more details.

 * You should have received a copy of the GNU General Public License
 * along with this program.  If not, see <http://www.gnu.org/licenses/>.

 * For the latest code and contact information visit: http://imcpwn.com
 */

package com.imcpwn.nofullbrightmining;

import java.util.ArrayList;
import java.util.List;
import java.util.logging.Logger;
import org.bukkit.ChatColor;
import org.bukkit.Location;
import org.bukkit.Material;
import org.bukkit.block.Block;
import org.bukkit.command.Command;
import org.bukkit.command.CommandSender;
import org.bukkit.entity.Player;
import org.bukkit.event.EventHandler;
import org.bukkit.event.EventPriority;
import org.bukkit.event.Listener;
import org.bukkit.event.block.BlockBreakEvent;
import org.bukkit.plugin.PluginDescriptionFile;
import org.bukkit.plugin.java.JavaPlugin;

public class NoFullBrightMining extends JavaPlugin implements Listener {

    public static NoFullBrightMining plugin;
    public final Logger logger = Logger.getLogger("Minecraft");

    // Global variables
    ArrayList<String> block_list = new ArrayList<String>();
    boolean perm_nfbm_disabled = false;
    // Configuration variables
    boolean nfbm_enabled;
    int minHeight;
    int minLightLevel;
    boolean dropChance_enabled;
    double dropPercent;
    boolean materials_blacklist;
    List<String> materials;

    @Override
    public void onDisable()
    {
        PluginDescriptionFile pdfFile = this.getDescription();
        this.logger.info(pdfFile.getName() + "is now disabled. ");
    }

    @Override
    public void onEnable()
    {
        PluginDescriptionFile pdfFile = this.getDescription();
        this.logger.info(pdfFile.getName() + "is now enabled. ");
        // Save default configuration
        this.saveDefaultConfig();
        // For EventListener
        getServer().getPluginManager().registerEvents(this, this);
        loadConfig();
    }

    public void loadConfig()
    {
        boolean temp_disabled = false;
        PluginDescriptionFile pdfFile = this.getDescription();
        this.reloadConfig();
        // Get configuration data from config.yml
        nfbm_enabled = this.getConfig().getBoolean("enabled");
        minHeight = this.getConfig().getInt("minimum-height");
        minLightLevel = this.getConfig().getInt("minimum-light-level");
        dropChance_enabled = this.getConfig().getBoolean("drop-chance-enabled");
        dropPercent = this.getConfig().getDouble("drop-percent");
        materials_blacklist = this.getConfig().getBoolean("materials-blacklist");
        materials = this.getConfig().getStringList("materials");

        // Check for invalid configuration
        // TODO: Fix this check.
        if (nfbm_enabled != true && nfbm_enabled != false)
        {
            this.logger.warning("Cannot load \"enabled\" configuration option! Default value is true.");
            temp_disabled = true;
            this.logger.warning(pdfFile.getName() + " could not be loaded and is now disabled. ");
        }
        // TODO: Fix this check.
        if (minHeight < 0 || minHeight > 256)
        {
            this.logger.warning("Cannot load \"minimumheight\" configuration option! Default value is 50.");
            temp_disabled = true;
            this.logger.warning(pdfFile.getName() + " could not be loaded and is now disabled. ");
        }
        // TODO: Fix this check.
        if (minLightLevel < 0 || minLightLevel > 15)
        {
            this.logger.warning("Cannot load \"mininumlightlevel\" configuration option! Default value is 4.");
            temp_disabled = true;
            this.logger.warning(pdfFile.getName() + " could not be loaded and is now disabled. ");
        }
        // TODO: Write check for materials_blacklist
        // TODO: Write check for dropChance_enabled
        // TODO: Write check for dropPercent
        // TODO: Fix this check.
        if (!(materials instanceof List))
        {
            this.logger.warning("Cannot load \"materials\" configuration option! Default value is DIAMOND_ORE as a list.");
            temp_disabled = true;
            this.logger.warning(pdfFile.getName() + " could not be loaded and is now disabled. ");
        }
        // Clear the old block list
        block_list.clear();
        // If temp_disabled is false, then don't disable the plugin
        if (temp_disabled)
        {
            perm_nfbm_disabled = true;
        }
        else
        {
            perm_nfbm_disabled = false;
            // Populate the block list with new materials
            for (String cm: materials)
            {
                block_list.add(cm);
            }
        }
    }

    @Override
    public boolean onCommand(CommandSender sender, Command cmd, String commandLabel, String[] args)
    {
        Player player = null;
        boolean is_player = false;
        final String enabledMSG = "NoFullBrightMining: enabled";
        final String disabledMSG = "NoFullBrightMining: disabled";
        final String reloadFailedMSG = "NoFullBrightMining: configuration reload failed. NoFullBrightMining is now disabled. Check console for details.";
        final String invalidArgMSG = "NoFullBrightMining: invalid argument count";
        final String reloadMSG = "NoFullBrightMining: reloaded";
        final String noPermissionMSG = "NFBM: No permission";
        if(sender instanceof Player)
        {
            player = (Player) sender;
            is_player = true;
        }
        else
        {
            is_player = false;
        }
        if (commandLabel.equalsIgnoreCase("nfbm"))
        {
            if (is_player)
            {
                if (player.hasPermission("nfbm.admin"))
                {
                    if (args.length == 0)
                    {
                        return false;
                    }
                    else if (args.length == 1)
                    {
                        if (args[0].equals("on"))
                        {
                            nfbm_enabled = true;
                            player.sendMessage(ChatColor.GREEN + enabledMSG);
                            return true;
                        }
                        else if (args[0].equals("off"))
                        {
                            nfbm_enabled = false;
                            player.sendMessage(ChatColor.RED + disabledMSG);
                            return true;
                        }
                        else if (args[0].equals("reload"))
                        {
                            loadConfig();
                            if (perm_nfbm_disabled)
                            {
                                player.sendMessage(ChatColor.RED + reloadFailedMSG);
                            }
                            else
                            {
                                player.sendMessage(ChatColor.GREEN + reloadMSG);
                            }
                            return true;
                        }
                        else if (args[0].equals("status"))
                        {
                            if (nfbm_enabled)
                            {
                                player.sendMessage(ChatColor.GREEN + enabledMSG);
                            }
                            else
                            {
                                player.sendMessage(ChatColor.RED + disabledMSG);
                            }
                            return true;
                        }
                        else
                        {
                            return false;
                        }
                    }
                    else
                    {
                        player.sendMessage(ChatColor.RED + invalidArgMSG);
                        return false;
                    }
                }
                else
                {
                    player.sendMessage(ChatColor.RED + noPermissionMSG);
                    return true;
                }
            }
            // If they are not a player
            else
            {
                if (sender.hasPermission("nfbm.admin"))
                {
                    if (args.length == 0)
                    {
                        return false;
                    }
                    else if (args.length == 1)
                    {
                        if (args[0].equals("on"))
                        {
                            nfbm_enabled = true;
                            sender.sendMessage(enabledMSG);
                            return true;
                        }
                        else if (args[0].equals("off"))
                        {
                            nfbm_enabled = false;
                            sender.sendMessage(disabledMSG);
                            return true;
                        }
                        else if (args[0].equals("reload"))
                        {
                            loadConfig();
                            if (perm_nfbm_disabled)
                            {
                                sender.sendMessage(reloadFailedMSG);
                            }
                            else
                            {
                                sender.sendMessage(reloadMSG);
                            }
                            return true;
                        }
                        else if (args[0].equals("status"))
                        {
                            if (nfbm_enabled)
                            {
                                sender.sendMessage(enabledMSG);
                            }
                            else
                            {
                                sender.sendMessage(disabledMSG);
                            }
                            return true;
                        }
                        else
                        {
                            return false;
                        }
                    }
                    else
                    {
                        sender.sendMessage(invalidArgMSG);
                        return false;
                    }
                }
                else
                {
                    sender.sendMessage(noPermissionMSG);
                    return true;
                }
            }
        }

        else if (commandLabel.equalsIgnoreCase("getll"))
        {
            if (is_player)
            {
                if (player.hasPermission("nfbm.getll"))
                {
                    // Source: https://bukkit.org/threads/getting-lightlevel-of-player.30950/
                    Location loc = player.getLocation();
                    int level = loc.getBlock().getLightLevel();
                    player.sendMessage("Your lightlevel is " + level);
                    return true;
                }
                else
                {
                    player.sendMessage(ChatColor.RED + noPermissionMSG);
                    return true;
                }
            }
            else
            {
                sender.sendMessage("You must be a player to run this command.");
                return true;
            }
        }
        return false;
    }

    // TODO: Improve this method. Specifically remove duplicate code (materals_blacklist options)
    // Source: https://bukkit.org/threads/block-break.109479/
    @EventHandler(ignoreCancelled=true, priority=EventPriority.LOWEST)
    public void onBlockBreak(BlockBreakEvent event)
    {
        if (nfbm_enabled && !perm_nfbm_disabled)
        {
            Player player = event.getPlayer();
            Location loc = player.getLocation();
            Block block = event.getBlock();
            double bYCoord = block.getY();
            boolean in_the_list = false;
            // Source: https://bukkit.org/threads/get-block-under-players-feet.117978/
            String block_name = block.getType().toString();
            final String errorMineMessage = ChatColor.RED + "Too dark to mine " + block_name + ". Try placing some torches.";
            // DO NOT USE BELOW. IT DOES NOT WORK.
            // int level = block.getLightLevel();
            int level = loc.getBlock().getLightLevel();
            if (bYCoord < minHeight)
            {
                for (int i = 0; i < block_list.size(); i++)
                {
                    if (block_name.equals(block_list.get(i)))
                    {
                        in_the_list = true;
                        break;
                    }
                }
                if (dropChance_enabled)
                {
                    double d = Math.random();
                    if (materials_blacklist)
                    {
                        if (in_the_list)
                        {
                            if (level < minLightLevel && !player.hasPermission("nfbm.bypass"))
                            {
                                // There is a dropPercent chance of the player receiving drops
                                if (d < dropPercent)
                                {

                                    player.sendMessage(errorMineMessage);
                                }
                                else
                                {
                                    event.setCancelled(true);
                                    block.setType(Material.AIR);
                                    player.sendMessage(errorMineMessage);
                                }
                            }
                        }
                    }
                    else
                    {
                        if (!in_the_list)
                        {
                            if (level < minLightLevel && !player.hasPermission("nfbm.bypass"))
                            {
                                // There is a dropPercent chance of the player receiving drops
                                if (d < dropPercent)
                                {

                                    player.sendMessage(errorMineMessage);
                                }
                                else
                                {
                                    event.setCancelled(true);
                                    block.setType(Material.AIR);
                                    player.sendMessage(errorMineMessage);
                                }
                            }
                        }
                    }
                }
                else // dropChance_enabled is false
                {
                    if (materials_blacklist)
                    {
                        if (in_the_list)
                        {
                            if (level < minLightLevel && !player.hasPermission("nfbm.bypass"))
                            {
                                event.setCancelled(true);
                                player.sendMessage(errorMineMessage);
                            }
                        }
                    }
                    else
                    {
                        if (!in_the_list)
                        {
                            if (level < minLightLevel && !player.hasPermission("nfbm.bypass"))
                            {
                                event.setCancelled(true);
                                player.sendMessage(errorMineMessage);
                            }
                        }
                    }
                }
            }
            else
            {
                // Block height is higher than the configured prevent breaking height
            }
        }
    }
}

plugin.yml

# NoFullBrightMining Bukkit/Spigot plugin by IMcPwn.
# Copyright (C) 2016 IMcPwn

# This program is free software: you can redistribute it and/or modify
# it under the terms of the GNU General Public License as published by
# the Free Software Foundation, either version 3 of the License, or
# (at your option) any later version.

# This program is distributed in the hope that it will be useful,
# but WITHOUT ANY WARRANTY; without even the implied warranty of
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
# GNU General Public License for more details.

# You should have received a copy of the GNU General Public License
# along with this program.  If not, see <http://www.gnu.org/licenses/>.

# For the latest code and contact information visit: http://imcpwn.com

name: NoFullBrightMining
version: 1.4
description: Prevent mining certain blocks based on light and height conditions
author: IMcPwn
main: com.imcpwn.nofullbrightmining.NoFullBrightMining
commands:
  nfbm:
    description: Main command for NoFullBrightMining administration
    usage: /<command> on|off|status|reload
  getll:
    description: Get player's current light level
    usage: /<command>
permissions:
  nfbm.bypass:
    description: Bypass NoFullBrightMining block break prevention
    default: op
  nfbm.admin:
    description: Control NoFullBrightMining in-game
    default: op
  nfbm.getll:
    description: Get player's current light level
    default: true

config.yml

# NoFullBrightMining Bukkit/Spigot plugin by IMcPwn.
# Copyright (C) 2016 IMcPwn

# This program is free software: you can redistribute it and/or modify
# it under the terms of the GNU General Public License as published by
# the Free Software Foundation, either version 3 of the License, or
# (at your option) any later version.

# This program is distributed in the hope that it will be useful,
# but WITHOUT ANY WARRANTY; without even the implied warranty of
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
# GNU General Public License for more details.

# You should have received a copy of the GNU General Public License
# along with this program.  If not, see <http://www.gnu.org/licenses/>.

# For the latest code and contact information visit: http://imcpwn.com

# Enable the plugin or not. 
# Can be toggled with /nfbm on and /nfbm off with permission nfbm.admin despite this setting.
# Options: [true/false]
enabled: true

# Minimum height to be ignored by this plugin.
# Options: [0-255]
# Example: 0 is bedrock, 255 is the sky.
minimum-height: 50

# Minimum light level to be ignored by this plugin.
# Options: [0-15]
# Example: 4 is night
minimum-light-level: 4

# Enable or disable block drop chance.
# If disabled blocks on the blacklist/not on the whitelist will be prevented from being broken.
# Options [true/false]
drop-chance-enabled: true

# Percent chance that blocks in the material list will have drops.
# drop-chance-enabled must be true for this to do anything!
# Options: [NUMBER]
# Example 0.60 is 60% chance of items on the blacklist/not on the whitelist dropping.
drop-percent: 0.60

# True makes the materials list the only blocks that are modified by this plugin based on previous configuration options. (BLACKLIST)
# False makes the materials list the only blocks that are not modified by this plugin based on previous configuration options. (WHITELIST)
# Options: [true/false]
materials-blacklist: true

# List of materials to be modified based on previous configuration options.
# NOTE: The larger this list is, the larger the impact the plugin will be on performance.
# Options: block names in list format
# Example: 
# - DIAMOND_ORE
# - IRON_ORE
# - EMERALD_ORE
materials:
  - DIAMOND_ORE

Answer 1

public static NoFullBrightMining plugin;

This static field seems unused, in which case it can be removed.

// Global variables
ArrayList<String> block_list = new ArrayList<String>();
boolean perm_nfbm_disabled = false;
// Configuration variables
boolean nfbm_enabled;

I'm not really sure why you have both a perm_nfbm_disabled and nfbm_enabled flag. Try to use only one. Or if they don't mean the same thing, give them names that make that fact clear.

int minHeight;
int minLightLevel;
boolean dropChance_enabled;
double dropPercent;
boolean materials_blacklist;
List<String> materials;

It seems all these variables are only used in this class, in which case they should be made private. You are also mixing naming conventions. Pick one and stick to it. The regular Java convention for variables is camelCase, so I suggest using that one.

onEnable

    // Save default configuration
    this.saveDefaultConfig();

This comment doesn't add much and can be removed (after all the code says the same thing). Perhaps it is worth mentioning why you are saving the configuration when the plugin gets enabled.

loadConfig

    if ((nfbm_enabled != true) && (nfbm_enabled != false)) {

This if statement will never be true.

    if ((minHeight < 0) || (minHeight > 256)) {
        logger.warning("Cannot load \"minimumheight\" configuration option! Default value is 50.");
        temp_disabled = true;
        logger.warning(pdfFile.getName()
                + " could not be loaded and is now disabled. ");
    }

Here you are giving a warning that the configuration is wrong, but are still continuing with the invalid value. Why not set the value to its default value?

    minHeight = DEFAULT_MINIMUM_HEIGHT;

Overriding the incorrect configuration with the default value would save you from having to keep track of the temp_disabled flag.

onCommand

    final String enabledMSG = "NoFullBrightMining: enabled";
    final String disabledMSG = "NoFullBrightMining: disabled";
    final String reloadFailedMSG = "NoFullBrightMining: configuration reload failed. NoFullBrightMining is now disabled. Check console for details.";
    final String invalidArgMSG = "NoFullBrightMining: invalid argument count";
    final String reloadMSG = "NoFullBrightMining: reloaded";
    final String noPermissionMSG = "NFBM: No permission";

These strings are all constants and should be defined at the beginning of your class. They should look like this:

    private static final String ENABLED_MESSAGE = "NoFullBrightMining: enabled";

Defining them early and making them static gives you less stuff to think about, which is always nice (especially when you're reading this code in six months).

The next piece of code is too long and contains too much stuff so lets start breaking it up. First let's create separate methods for both of the commands you're accepting.

private boolean onNfbmCommand(CommandSender sender, String[] args) {
}   

private boolean onGetllCommand(CommandSender sender) {
} 

The onCommand method will now look something like this:

@Override
public boolean onCommand(CommandSender sender, Command cmd,
        String commandLabel, String[] args) {
    if (commandLabel.equalsIgnoreCase("nfbm")) {
        return onNfbmCommand(sender, args);
    }
    else if (commandLabel.equalsIgnoreCase("getll")) {
        return onGetllCommand(sender);
    }
    return false;
}

Notice how all it is doing is passing the call on the the other methods when it is appropriate.

onNfbmCommand

Let's look at the onNfbmCommand Method. This part is difficult because it contains a very large nested if statement. This makes it hard to think about, so let's see what we can simplify.

It seems both branches are actually almost the same, except for the specific message they are sending. In other words, it doesn't matter whether the sender is a player or not. We can use that to our advantage!

First lets create a method that sends a message to the sender with a color if it's a Player and without a color if it's not:

private void sendMessage(CommandSender sender, String message, ChatColor color) {
    if (sender instanceof Player) {
        sender.sendMessage(color + message);
    } else {
        sender.sendMessage(message);
    }
}

We can now use this method in the onNfbmCommand method and we don't have to worry about the difference between player and sender anymore. Hurray, that's one less if-statement to worry about!

private boolean onNfbmCommand(CommandSender sender, String[] args) {

But we're still stuck with quite a big nested if-statement. To get rid of this nesting we can use something called a guard clause. Which basically means we're checking things as early as possible so we can return quickly. So in this case we can do this:

    if (!sender.hasPermission("nfbm.admin")) {
        sendMessage(sender, noPermissionMSG, ChatColor.RED);
        return true;
    }

If the sender doesn't have permission we send a message and return quickly. We don't have to worry about permission for the rest of the method! Let's do the rest too. It looks something like this:

    if (args.length == 0) {
        return false;
    } 
    if (args.length > 1) {
        sendMessage(sender, invalidArgMSG, ChatColor.RED);
        return false;
    }
    String argument = args[0];
    if (argument.equals("on")) {
        nfbm_enabled = true;
        sendMessage(sender, enabledMSG, ChatColor.GREEN);
        return true;
    } 
    if (argument.equals("off")) {
        nfbm_enabled = false;
        sendMessage(sender, disabledMSG, ChatColor.RED);
        return true;
    }
    if (argument.equals("reload")) {
        loadConfig();
        if (perm_nfbm_disabled) {
            sendMessage(sender, reloadFailedMSG, ChatColor.RED);
        } else {
            sendMessage(sender, reloadMSG, ChatColor.GREEN);
        }
        return true;
    }
    if (argument.equals("status")) {
        if (nfbm_enabled) {
            sendMessage(sender, enabledMSG, ChatColor.GREEN);
        } else {
            sendMessage(sender, disabledMSG, ChatColor.RED);
        }
        return true;
    } 
    return false;
}

By breaking down the nested if-statement it becomes much easier to think about. Now we can add new stuff and fix broken stuff much faster.

onGetLLCommand

This one is a bit easier.

public boolean onGetLLCommand(CommandSender sender) {

We quickly get rid of cases we don't need:

    if (!(sender instanceof Player)) {
        sender.sendMessage("You must be a player to run this command.");
        return true;
    } 

Now the rest of the method looks like this:

    Player player = (Player) sender;
    if (!player.hasPermission("nfbm.getll")) {
        player.sendMessage(ChatColor.RED + noPermissionMSG);
        return true;    
    }   

    // Source:
    // https://bukkit.org/threads/getting-lightlevel-of-player.30950/
    Location loc = player.getLocation();
    int level = loc.getBlock().getLightLevel();
    player.sendMessage("Your lightlevel is " + level);
    return true;
}

onBlockBreak

@EventHandler(ignoreCancelled = true, priority = EventPriority.LOWEST)
public void onBlockBreak(BlockBreakEvent event) {

Again we'll get rid of cases we don't need.

if (!nfbm_enabled || perm_nfbm_disabled) {
    return;
}

Let's define the variables of this method.

    final Block block = event.getBlock();
    final Player player = event.getPlayer();
    boolean isMinable = isMinable(player, block);
    final String errorMineMessage = ChatColor.RED + "Too dark to mine "
            + block.getType().toString() + ". Try placing some torches.";

I've changed a couple of things:

• Renamed all variables to follow camelCase.
• Removed the location variable, because it's only used in one place.
• Removed the block_name variable.
• Removed the light_level variable.
• Created an isMinable variable and set it using a method we'll define in a bit.
• Removed the is_in_list variable. We don't really care whether a block is in a list here, we care whether it can be mined.

The most important thing we've done is create a seperate method that will check whether a block is minable. By using a seperate method, we can focus the onBlockBreak method on 'what to do' instead of deciding 'when to do it'. The rest of the method now looks like this:

    if (isMinable) {
        return;
    }
    if (dropChance_enabled) {
        double d = Math.random();
        if (d < dropPercent) {
            // lucky!
            player.sendMessage(errorMineMessage);
            return;
        }
    }
    // Can't mine this
    event.setCancelled(true);
    block.setType(Material.AIR);
    player.sendMessage(errorMineMessage);
}

Notice how it doesn't have to care anymore about whether the block is blacklisted, or the current lightlevel, or any of that stuff, we moved all that to the isMinable method. This allows the onBlockBreak method to focus on one thing, and makes it much easier to read!

The isMinable method looks like this:

private boolean isMinable(Player player, Block block) {
    if (block.getY() >= minHeight) {
        // Block height is higher than the configured prevent breaking
        // height
        return true;
    }
    if (player.hasPermission("nfbm.bypass")) {
        // Player has bypass permission.
        return true;
    }

    boolean isInList = block_list.contains(block.getType().toString());
    if (materials_blacklist && !isInList) {
        // Block not in blacklist.
        return true;
    }
    if (!materials_blacklist && isInList) {
        // Block in whitelist.
        return true;
    }

    int lightLevel = player.getLocation().getBlock().getLightLevel();
    return lightLevel < minLightLevel;
}

Again, we're determining whether a block is minable as early as possible. This prevent deep nesting if-statements and prevents repetition. This is the list of checks:

• If a block is higher than the minimum height, it is always minable.
• If the player has bypass permission, it is minable
• If the block is not in the blocklist and the list is a blacklist, the block is minable.
• If the block is in the blocklist and the list is a whitelist, the block is minable.
• If the lightlevel is above the minLightLevel, the block is minable.

Something I didn't fix, but you definitely should, is the use of a single list as both a black- and white-list. It is much easier to create two different lists, and check each of them seperately. because now you have to keep thinking 'is this list a white-list or a black-list?'.

Conclusion

Methods should focus on one thing as much as possible. By seperating code into methods we reduce duplication and increase clarity.

Lastly, by preferring early returns we prevent deep nested if-statements, again reducing duplication and increasing clarity.