Fixed some code smells (#698)

* WIP code clean up and code smell removal

* Tried to fix a few more code smells

src/main/java/world/bentobox/bentobox/api/addons/AddonClassLoader.java

@@ -45,6 +45,9 @@ public class AddonClassLoader extends URLClassLoader {
         Class<?> javaClass;
         try {
             String mainClass = data.getString("main");
+            if (mainClass == null) {
+                throw new InvalidAddonFormatException("addon.yml does not define a main class!");
+            }
             javaClass = Class.forName(mainClass, true, this);
             if(mainClass.startsWith("world.bentobox.bentobox")){
                 throw new InvalidAddonFormatException("Package declaration cannot start with 'world.bentobox.bentobox'");
@@ -90,15 +93,15 @@ public class AddonClassLoader extends URLClassLoader {
                 .metrics(data.getBoolean("metrics", true))
                 .repository(data.getString("repository", ""));
 
-        if (data.getString("depend") != null) {
+        String depend = data.getString("depend");
-            builder.dependencies(Arrays.asList(data.getString("depend").split("\\s*,\\s*")));
+        if (depend != null) {
+            builder.dependencies(Arrays.asList(depend.split("\\s*,\\s*")));
         }
-        if (data.getString("softdepend") != null) {
+        String softDepend = data.getString("softdepend");
-            builder.softDependencies(Arrays.asList(data.getString("softdepend").split("\\s*,\\s*")));
+        if (softDepend != null) {
-        }
+            builder.softDependencies(Arrays.asList(softDepend.split("\\s*,\\s*")));
-        if (data.getString("icon") != null) {
-            builder.icon(Material.getMaterial(data.getString("icon", "PAPER")));
         }
+        builder.icon(Material.getMaterial(data.getString("icon", "PAPER")));
 
         return builder.build();
     }

src/main/java/world/bentobox/bentobox/api/commands/admin/resets/AdminResetsResetCommand.java

@@ -1,9 +1,6 @@
 package world.bentobox.bentobox.api.commands.admin.resets;
 
-import java.util.Arrays;
+import java.util.*;
-import java.util.List;
-import java.util.Optional;
-import java.util.UUID;
 
 import org.bukkit.Bukkit;
 import org.bukkit.entity.Player;
@@ -58,6 +55,6 @@ public class AdminResetsResetCommand extends ConfirmableCommand {
 
     @Override
     public Optional<List<String>> tabComplete(User user, String alias, List<String> args) {
-        return Optional.of(Arrays.asList("@a"));
+        return Optional.of(Collections.singletonList("@a"));
     }
 }

src/main/java/world/bentobox/bentobox/api/commands/island/IslandNearCommand.java

@@ -18,7 +18,7 @@ import world.bentobox.bentobox.database.objects.Island;
  */
 public class IslandNearCommand extends CompositeCommand {
 
-    private final static List<BlockFace> COMPASS_POINTS = Arrays.asList(BlockFace.NORTH, BlockFace.EAST, BlockFace.SOUTH, BlockFace.WEST);
+    private static final List<BlockFace> COMPASS_POINTS = Arrays.asList(BlockFace.NORTH, BlockFace.EAST, BlockFace.SOUTH, BlockFace.WEST);
 
     public IslandNearCommand(CompositeCommand islandCommand) {
         super(islandCommand, "near");

src/main/java/world/bentobox/bentobox/blueprints/Blueprint.java

@@ -1,6 +1,3 @@
-/**
- *
- */
 package world.bentobox.bentobox.blueprints;
 
 import java.util.ArrayList;
@@ -21,7 +18,7 @@ import world.bentobox.bentobox.blueprints.dataobjects.BlueprintEntity;
  * @author tastybento
  *
  */
-public class Blueprint implements Comparable<Blueprint> {
+public class Blueprint {
 
     /**
      * Unique name for this blueprint. The filename will be this plus the blueprint suffix
@@ -197,9 +194,5 @@ public class Blueprint implements Comparable<Blueprint> {
     public void setBedrock(Vector bedrock) {
         this.bedrock = bedrock;
     }
-    @Override
-    public int compareTo(Blueprint o) {
-        return this.name.compareToIgnoreCase(o.name);
-    }
 
 }

src/main/java/world/bentobox/bentobox/blueprints/BlueprintClipboard.java

@@ -96,6 +96,9 @@ public class BlueprintClipboard {
 
         // World
         World world = pos1.getWorld();
+        if (world == null) {
+            return false;
+        }
         // Clear the clipboard
         blueprint = new Blueprint();
 
@@ -196,18 +199,18 @@ public class BlueprintClipboard {
             if (entity instanceof ChestedHorse) {
                 bpe.setChest(((ChestedHorse)entity).isCarryingChest());
             }
-            if (entity instanceof Ageable) {
+            // Only set if child. Most animals are adults
-                // Only set if child. Most animals are adults
+            if (entity instanceof Ageable && !((Ageable)entity).isAdult()) {
-                if (!((Ageable)entity).isAdult()) bpe.setAdult(false);
+                bpe.setAdult(false);
             }
             if (entity instanceof AbstractHorse) {
                 AbstractHorse horse = (AbstractHorse)entity;
                 bpe.setDomestication(horse.getDomestication());
                 bpe.setInventory(new HashMap<>());
-                for (int index = 0; index < horse.getInventory().getSize(); index++) {
+                for (int i = 0; i < horse.getInventory().getSize(); i++) {
-                    ItemStack i = horse.getInventory().getItem(index);
+                    ItemStack item = horse.getInventory().getItem(i);
-                    if (i != null) {
+                    if (item != null) {
-                        bpe.getInventory().put(index, i);
+                        bpe.getInventory().put(i, item);
                     }
                 }
             }
@@ -259,10 +262,10 @@ public class BlueprintClipboard {
         if (blockState instanceof InventoryHolder) {
             b.setInventory(new HashMap<>());
             InventoryHolder ih = (InventoryHolder)blockState;
-            for (int index = 0; index < ih.getInventory().getSize(); index++) {
+            for (int i = 0; index < ih.getInventory().getSize(); i++) {
-                ItemStack i = ih.getInventory().getItem(index);
+                ItemStack item = ih.getInventory().getItem(i);
-                if (i != null) {
+                if (item != null) {
-                    b.getInventory().put(index, i);
+                    b.getInventory().put(i, item);
                 }
             }
         }

src/main/java/world/bentobox/bentobox/blueprints/BlueprintPaster.java

@@ -58,7 +58,7 @@ public class BlueprintPaster {
 
     private static final String MINECRAFT = "minecraft:";
 
-    private Map<String, String> BLOCK_CONVERSION = ImmutableMap.of("sign", "oak_sign", "wall_sign", "oak_wall_sign");
+    private static final Map<String, String> BLOCK_CONVERSION = ImmutableMap.of("sign", "oak_sign", "wall_sign", "oak_wall_sign");
 
     private BentoBox plugin;
     // The minimum block position (x,y,z)

src/main/java/world/bentobox/bentobox/blueprints/conversation/DescriptionPrompt.java

@@ -21,6 +21,7 @@ import world.bentobox.bentobox.blueprints.dataobjects.BlueprintBundle;
  */
 public class DescriptionPrompt extends StringPrompt {
 
+    private static final String DESCRIPTION = "description";
     private GameModeAddon addon;
     private BlueprintBundle bb;
 
@@ -33,9 +34,9 @@ public class DescriptionPrompt extends StringPrompt {
     @Override
     public String getPromptText(ConversationContext context) {
         User user = User.getInstance((Player)context.getForWhom());
-        if (context.getSessionData("description") != null) {
+        if (context.getSessionData(DESCRIPTION) != null) {
             StringBuilder sb = new StringBuilder();
-            for (String line : ((List<String>) context.getSessionData("description"))) {
+            for (String line : ((List<String>) context.getSessionData(DESCRIPTION))) {
                 sb.append(user.getTranslation("commands.admin.blueprint.management.description.default-color"));
                 sb.append(line);
                 sb.append(System.getProperty("line.separator"));
@@ -53,11 +54,11 @@ public class DescriptionPrompt extends StringPrompt {
             return new DescriptionSuccessPrompt(addon, bb);
         }
         List<String> desc = new ArrayList<>();
-        if (context.getSessionData("description") != null) {
+        if (context.getSessionData(DESCRIPTION) != null) {
-            desc = ((List<String>) context.getSessionData("description"));
+            desc = ((List<String>) context.getSessionData(DESCRIPTION));
         }
         desc.add(ChatColor.translateAlternateColorCodes('&', input));
-        context.setSessionData("description", desc);
+        context.setSessionData(DESCRIPTION, desc);
         return this;
     }
 }

src/main/java/world/bentobox/bentobox/blueprints/dataobjects/BlueprintBundle.java

@@ -1,16 +1,15 @@
 package world.bentobox.bentobox.blueprints.dataobjects;
 
+import com.google.gson.annotations.Expose;
+import org.bukkit.Material;
+import org.bukkit.World;
+import world.bentobox.bentobox.blueprints.Blueprint;
+import world.bentobox.bentobox.database.objects.DataObject;
+
 import java.util.ArrayList;
 import java.util.EnumMap;
 import java.util.List;
-
+import java.util.Map;
-import org.bukkit.Material;
-import org.bukkit.World;
-
-import com.google.gson.annotations.Expose;
-
-import world.bentobox.bentobox.blueprints.Blueprint;
-import world.bentobox.bentobox.database.objects.DataObject;
 
 /**
  * Represents a bundle of three {@link Blueprint}s.
@@ -52,7 +51,7 @@ public class BlueprintBundle implements DataObject {
      * Reference to the blueprint
      */
     @Expose
-    private EnumMap<World.Environment, String> blueprints = new EnumMap<>(World.Environment.class);
+    private Map<World.Environment, String> blueprints = new EnumMap<>(World.Environment.class);
     /**
      * @return the uniqueId
      */
@@ -106,19 +105,19 @@ public class BlueprintBundle implements DataObject {
     /**
      * @return the blueprints
      */
-    public EnumMap<World.Environment, String> getBlueprints() {
+    public Map<World.Environment, String> getBlueprints() {
         return blueprints;
     }
     /**
      * @param blueprints the blueprints to set
      */
-    public void setBlueprints(EnumMap<World.Environment, String> blueprints) {
+    public void setBlueprints(Map<World.Environment, String> blueprints) {
         this.blueprints = blueprints;
     }
 
     /**
      * Adds a blueprint to the blueprint bundle. It will replace any blueprint that already exists of the same {@link World.Environment} type.
-     * @param env - the {@link World#Environment}
+     * @param env - the {@link World.Environment}
      * @param bp - blueprint
      */
     public void setBlueprint(World.Environment env, Blueprint bp) {
@@ -135,7 +134,7 @@ public class BlueprintBundle implements DataObject {
 
     /**
      * Get the blueprint for the environment type
-     * @param env - {@link World#Environment} type
+     * @param env - {@link World.Environment} type
      * @return Blueprint or null if one does not exist
      */
     public String getBlueprint(World.Environment env) {

src/main/java/world/bentobox/bentobox/database/json/BentoboxTypeAdapterFactory.java

@@ -1,6 +1,3 @@
-/**
- *
- */
 package world.bentobox.bentobox.database.json;
 
 import java.util.Map;

src/main/java/world/bentobox/bentobox/database/json/JSONDatabaseHandler.java

@@ -103,7 +103,7 @@ public class JSONDatabaseHandler<T> extends AbstractJSONDatabaseHandler<T> {
         // Obtain the value of uniqueId within the instance (which must be a DataObject)
         PropertyDescriptor propertyDescriptor = new PropertyDescriptor("uniqueId", dataObject);
         Method method = propertyDescriptor.getReadMethod();
-        String fileName = (String) method.invoke(instance) + JSON;
+        String fileName = method.invoke(instance) + JSON;
 
         File tableFolder = new File(plugin.getDataFolder(), path);
         File file = new File(tableFolder, fileName);

src/main/java/world/bentobox/bentobox/database/json/adapters/ItemStackTypeAdapter.java

@@ -1,6 +1,3 @@
-/**
- *
- */
 package world.bentobox.bentobox.database.json.adapters;
 
 import java.io.IOException;

src/main/java/world/bentobox/bentobox/database/mariadb/MariaDBDatabaseHandler.java

@@ -121,12 +121,11 @@ public class MariaDBDatabaseHandler<T> extends AbstractJSONDatabaseHandler<T> {
 
     private List<T> loadIt(Statement preparedStatement) {
         List<T> list = new ArrayList<>();
-        StringBuilder sb = new StringBuilder();
-        sb.append("SELECT `json` FROM `");
-        sb.append(dataObject.getCanonicalName());
-        sb.append("`");
 
-        try (ResultSet resultSet = preparedStatement.executeQuery(sb.toString())) {
+        String sb = "SELECT `json` FROM `" +
+                dataObject.getCanonicalName() +
+                "`";
+        try (ResultSet resultSet = preparedStatement.executeQuery(sb)) {
             // Load all the results
             Gson gson = getGson();
             while (resultSet.next()) {

src/main/java/world/bentobox/bentobox/database/mysql/MySQLDatabaseHandler.java

@@ -121,12 +121,11 @@ public class MySQLDatabaseHandler<T> extends AbstractJSONDatabaseHandler<T> {
 
     private List<T> loadIt(Statement preparedStatement) {
         List<T> list = new ArrayList<>();
-        StringBuilder sb = new StringBuilder();
-        sb.append("SELECT `json` FROM `");
-        sb.append(dataObject.getCanonicalName());
-        sb.append("`");
 
-        try (ResultSet resultSet = preparedStatement.executeQuery(sb.toString())) {
+        String sb = "SELECT `json` FROM `" +
+                dataObject.getCanonicalName() +
+                "`";
+        try (ResultSet resultSet = preparedStatement.executeQuery(sb)) {
             // Load all the results
             Gson gson = getGson();
             while (resultSet.next()) {

src/main/java/world/bentobox/bentobox/database/objects/IslandDeletion.java

@@ -81,13 +81,8 @@ public class IslandDeletion implements DataObject {
         }
         IslandDeletion other = (IslandDeletion) obj;
         if (uniqueId == null) {
-            if (other.uniqueId != null) {
+            return other.uniqueId == null;
-                return false;
+        } else return uniqueId.equals(other.uniqueId);
-            }
-        } else if (!uniqueId.equals(other.uniqueId)) {
-            return false;
-        }
-        return true;
     }
 
     /**

src/main/java/world/bentobox/bentobox/listeners/flags/clicklisteners/CommandCycleClick.java

@@ -1,6 +1,3 @@
-/**
- *
- */
 package world.bentobox.bentobox.listeners.flags.clicklisteners;
 
 import org.bukkit.Sound;

src/main/java/world/bentobox/bentobox/listeners/flags/clicklisteners/CommandRankClickListener.java

@@ -1,6 +1,3 @@
-/**
- *
- */
 package world.bentobox.bentobox.listeners.flags.clicklisteners;
 
 import java.util.ArrayList;

src/main/java/world/bentobox/bentobox/listeners/flags/clicklisteners/GeoLimitClickListener.java

@@ -1,6 +1,3 @@
-/**
- *
- */
 package world.bentobox.bentobox.listeners.flags.clicklisteners;
 
 import java.util.Arrays;

src/main/java/world/bentobox/bentobox/managers/AddonsManager.java

@@ -108,9 +108,7 @@ public class AddonsManager {
         Bukkit.getPluginManager().callEvent(new AddonEvent().builder().addon(addon).reason(AddonEvent.Reason.LOAD).build());
 
         // Add it to the list of addons
-        if (addons.contains(addon)) {
+        addons.remove(addon);
-            addons.remove(addon);
-        }
         addons.add(addon);
 
         // Add to the list of loaders

src/main/java/world/bentobox/bentobox/managers/BlueprintsManager.java

@@ -85,13 +85,9 @@ public class BlueprintsManager {
         .enableComplexMapKeySerialization()
         .setPrettyPrinting()
         // This enables gson to deserialize enum maps
-        .registerTypeAdapter(EnumMap.class, new InstanceCreator<EnumMap>() {
+        .registerTypeAdapter(EnumMap.class, (InstanceCreator<EnumMap>) type -> {
-            @SuppressWarnings("unchecked")
+            Type[] types = (((ParameterizedType) type).getActualTypeArguments());
-            @Override
+            return new EnumMap((Class<?>) types[0]);
-            public EnumMap createInstance(Type type) {
-                Type[] types = (((ParameterizedType) type).getActualTypeArguments());
-                return new EnumMap((Class<?>) types[0]);
-            }
         });
         // Disable <>'s escaping etc.
         builder.disableHtmlEscaping();

src/main/java/world/bentobox/bentobox/managers/IslandsManager.java

@@ -741,7 +741,7 @@ public class IslandsManager {
         quarantineCache.clear();
         List<Island> toQuarantine = new ArrayList<>();
         // Attempt to load islands
-        handler.loadObjects().stream().forEach(island -> {
+        handler.loadObjects().forEach(island -> {
             if (island.isDeleted()) {
                 // These will be deleted later
                 deletedIslands.add(island.getUniqueId());
@@ -927,7 +927,7 @@ public class IslandsManager {
 
     public void shutdown(){
         // Remove all coop associations
-        islandCache.getIslands().stream().forEach(i -> i.getMembers().values().removeIf(p -> p == RanksManager.COOP_RANK));
+        islandCache.getIslands().forEach(i -> i.getMembers().values().removeIf(p -> p == RanksManager.COOP_RANK));
         saveAll();
         islandCache.clear();
         handler.close();
@@ -1004,7 +1004,7 @@ public class IslandsManager {
      * @param uniqueId - UUID of player
      */
     public void clearRank(int rank, UUID uniqueId) {
-        islandCache.getIslands().stream().forEach(i -> i.getMembers().entrySet().removeIf(e -> e.getKey().equals(uniqueId) && e.getValue() == rank));
+        islandCache.getIslands().forEach(i -> i.getMembers().entrySet().removeIf(e -> e.getKey().equals(uniqueId) && e.getValue() == rank));
     }
 
     /**

src/main/java/world/bentobox/bentobox/managers/LocalesManager.java

@@ -36,7 +36,7 @@ public class LocalesManager {
 
     public LocalesManager(BentoBox plugin) {
         this.plugin = plugin;
-        copyLocalesFromPluginJar(BENTOBOX);
+        copyLocalesFromPluginJar();
         loadLocalesFromFile(BENTOBOX); // Default
     }
 
@@ -126,11 +126,10 @@ public class LocalesManager {
     /**
      * Copies all the locale files from the plugin jar to the filesystem.
      * Only done if the locale folder does not already exist.
-     * @param folderName - the name of the destination folder
      */
-    private void copyLocalesFromPluginJar(String folderName) {
+    private void copyLocalesFromPluginJar() {
         // Run through the files and store the locales
-        File localeDir = new File(plugin.getDataFolder(), LOCALE_FOLDER + File.separator + folderName);
+        File localeDir = new File(plugin.getDataFolder(), LOCALE_FOLDER + File.separator + LocalesManager.BENTOBOX);
         // If the folder does not exist, then make it and fill with the locale files from the jar
         // If it does exist, then new files will NOT be written!
         if (!localeDir.exists()) {
@@ -232,7 +231,7 @@ public class LocalesManager {
      */
     public void reloadLanguages() {
         languages.clear();
-        copyLocalesFromPluginJar(BENTOBOX);
+        copyLocalesFromPluginJar();
         loadLocalesFromFile(BENTOBOX);
         plugin.getAddonsManager().getAddons().forEach(addon -> {
             copyLocalesFromAddonJar(addon);

src/main/java/world/bentobox/bentobox/panels/BlueprintManagementPanel.java

@@ -263,10 +263,6 @@ public class BlueprintManagementPanel {
                 .build();
     }
 
-    public boolean trashBundle(Panel panel, User user, ClickType clickType, int slot ) {
-        return true;
-    }
-
     public void askForName(Conversable whom, GameModeAddon addon) {
         new ConversationFactory(BentoBox.getInstance())
         .withModality(true)

src/main/java/world/bentobox/bentobox/panels/BundleManagementPanel.java

@@ -1,14 +0,0 @@
-package world.bentobox.bentobox.panels;
-
-import org.bukkit.event.inventory.ClickType;
-
-import world.bentobox.bentobox.api.panels.Panel;
-import world.bentobox.bentobox.api.user.User;
-
-public class BundleManagementPanel {
-    private BundleManagementPanel() {}
-
-    public static boolean openPanel(Panel panel, User user, ClickType clickType, int slot ) {
-        return true;
-    }
-}

src/main/java/world/bentobox/bentobox/schems/Converter.java

@@ -120,7 +120,7 @@ public class Converter {
             ConfigurationSection inv = config.getConfigurationSection(INVENTORY);
             block.setInventory(
                     inv.getKeys(false).stream()
-                    .collect(Collectors.toMap(i -> Integer.valueOf(i), i -> (ItemStack)inv.get(i)))
+                    .collect(Collectors.toMap(Integer::valueOf, i -> (ItemStack)inv.get(i)))
                     );
         }
         // Mob spawners
@@ -194,7 +194,7 @@ public class Converter {
             if (cs.isConfigurationSection(INVENTORY)) {
                 ConfigurationSection inv = cs.getConfigurationSection(INVENTORY);
                 be.setInventory(inv.getKeys(false).stream()
-                        .collect(Collectors.toMap(i -> Integer.valueOf(i), i ->  (ItemStack)inv.get(i))));
+                        .collect(Collectors.toMap(Integer::valueOf, i ->  (ItemStack)inv.get(i))));
             }
             return be;
         } catch (Exception e) {

src/main/java/world/bentobox/bentobox/util/teleport/SafeSpotTeleport.java

@@ -225,7 +225,7 @@ public class SafeSpotTeleport {
             for (int z = 0; z < 16; z++) {
                 // Work down from the entry point up
                 for (int y = Math.min(chunk.getHighestBlockYAt(x, z), MAX_HEIGHT); y >= 0; y--) {
-                    if (checkBlock(chunk, x,y,z, MAX_HEIGHT)) {
+                    if (checkBlock(chunk, x,y,z)) {
                         return true;
                     }
                 } // end y
@@ -266,15 +266,14 @@ public class SafeSpotTeleport {
      * @param x - x coordinate
      * @param y - y coordinate
      * @param z - z coordinate
-     * @param worldHeight - height of world
      * @return true if this is a safe spot, false if this is a portal scan
      */
-    private boolean checkBlock(ChunkSnapshot chunk, int x, int y, int z, int worldHeight) {
+    private boolean checkBlock(ChunkSnapshot chunk, int x, int y, int z) {
         World world = location.getWorld();
         Material type = chunk.getBlockType(x, y, z);
         if (!type.equals(Material.AIR)) { // AIR
-            Material space1 = chunk.getBlockType(x, Math.min(y + 1, worldHeight), z);
+            Material space1 = chunk.getBlockType(x, Math.min(y + 1, SafeSpotTeleport.MAX_HEIGHT), z);
-            Material space2 = chunk.getBlockType(x, Math.min(y + 2, worldHeight), z);
+            Material space2 = chunk.getBlockType(x, Math.min(y + 2, SafeSpotTeleport.MAX_HEIGHT), z);
             if ((space1.equals(Material.AIR) && space2.equals(Material.AIR)) || (space1.equals(Material.NETHER_PORTAL) && space2.equals(Material.NETHER_PORTAL))
                     && (!type.toString().contains("FENCE") && !type.toString().contains("DOOR") && !type.toString().contains("GATE") && !type.toString().contains("PLATE")
                     && !type.toString().contains("SIGN"))) {