Some code cleanup (#2414)

Author: Photon-GitHub

src/main/java/com/comphenix/protocol/CommandFilter.java

@@ -1,17 +1,10 @@
 package com.comphenix.protocol;
 
-import java.util.ArrayList;
-import java.util.Deque;
-import java.util.HashSet;
-import java.util.Iterator;
-import java.util.List;
-import java.util.Set;
-
-import javax.script.Invocable;
-import javax.script.ScriptEngine;
-import javax.script.ScriptEngineManager;
-import javax.script.ScriptException;
-
+import com.comphenix.protocol.MultipleLinesPrompt.MultipleConversationCanceller;
+import com.comphenix.protocol.error.ErrorReporter;
+import com.comphenix.protocol.error.Report;
+import com.comphenix.protocol.error.ReportType;
+import com.comphenix.protocol.events.PacketEvent;
 import org.bukkit.ChatColor;
 import org.bukkit.command.CommandSender;
 import org.bukkit.conversations.Conversable;
@@ -22,11 +15,16 @@
 import org.bukkit.conversations.ConversationFactory;
 import org.bukkit.plugin.Plugin;
 
-import com.comphenix.protocol.MultipleLinesPrompt.MultipleConversationCanceller;
-import com.comphenix.protocol.error.ErrorReporter;
-import com.comphenix.protocol.error.Report;
-import com.comphenix.protocol.error.ReportType;
-import com.comphenix.protocol.events.PacketEvent;
+import javax.script.Invocable;
+import javax.script.ScriptEngine;
+import javax.script.ScriptEngineManager;
+import javax.script.ScriptException;
+import java.util.Deque;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.Iterator;
+import java.util.Map;
+import java.util.Set;
 
 /**
  * A command to apply JavaScript filtering to the packet command.
@@ -48,7 +46,7 @@ public interface FilterFailedHandler{
          * @param ex - the failure.
          * @return TRUE to keep processing this filter, FALSE to remove it.
          */
-        public boolean handle(PacketEvent event, Filter filter, Exception ex);
+        boolean handle(PacketEvent event, Filter filter, Exception ex);
     }
 
     /**
@@ -57,7 +55,7 @@ public interface FilterFailedHandler{
      * @author Kristian
      */
     private enum SubCommand {
-        ADD, REMOVE;
+        ADD, REMOVE
     }
 
     /**
@@ -206,13 +204,13 @@ public CompilationSuccessCanceller clone() {
     private FilterFailedHandler defaultFailedHandler;
 
     // Currently registered filters
-    private List<Filter> filters = new ArrayList<Filter>();
+    private final Map<String, Filter> filters = new HashMap<>();
 
     // Owner plugin
     private final Plugin plugin;
 
-    // Whether or not the command is enabled
-    private ProtocolConfig config;
+    // Whether the command is enabled
+    private final ProtocolConfig config;
 
     // Script engine
     private ScriptEngine engine;
@@ -227,7 +225,7 @@ public CommandFilter(ErrorReporter reporter, Plugin plugin, ProtocolConfig confi
         this.uninitialized = true;
     }
 
-    private void initalizeScript() {
+    private void initializeScript() {
         try {
             // First attempt
             initializeEngine();
@@ -306,7 +304,7 @@ public boolean handle(PacketEvent event, Filter filter, Exception ex) {
     }
 
     /**
-     * Determine whether or not to pass the given packet event to the packet listeners.
+     * Determine whether to pass the given packet event to the packet listeners.
      * <p>
      * Uses a default filter failure handler that simply prints the error message and removes the filter.
      * @param event - the event.
@@ -317,13 +315,13 @@ public boolean filterEvent(PacketEvent event) {
     }
 
     /**
-     * Determine whether or not to pass the given packet event to the packet listeners.
+     * Determine whether to pass the given packet event to the packet listeners.
      * @param event - the event.
      * @param handler - failure handler.
      * @return TRUE if we should, FALSE otherwise.
      */
     public boolean filterEvent(PacketEvent event, FilterFailedHandler handler) {
-        for (Iterator<Filter> it = filters.iterator(); it.hasNext(); ) {
+        for (Iterator<Filter> it = filters.values().iterator(); it.hasNext(); ) {
             Filter filter = it.next();
 
             try {
@@ -347,7 +345,7 @@ private void checkScriptStatus() {
         // Start the engine
         if (uninitialized) {
             uninitialized = false;
-            initalizeScript();
+            initializeScript();
         }
     }
 
@@ -370,11 +368,12 @@ protected boolean handleCommand(CommandSender sender, String[] args) {
 
         final SubCommand command = parseCommand(args, 0);
         final String name = args[1];
-        
+        final String lowerCaseName = name.toLowerCase();
+
         switch (command) {
             case ADD:
                 // Never overwrite an existing filter
-                if (findFilter(name) != null) {
+                if(filters.containsKey(lowerCaseName)) {
                     sender.sendMessage(ChatColor.RED + "Filter " + name + " already exists. Remove it first.");
                     return true;
                 }
@@ -402,8 +401,8 @@ public void conversationAbandoned(ConversationAbandonedEvent event) {
                                     final Conversable whom = event.getContext().getForWhom();
 
                                     if (event.gracefulExit()) {
-                                        String predicate = prompt.removeAccumulatedInput(event.getContext());
-                                        Filter filter = new Filter(name, predicate, packets);
+                                        final String predicate = prompt.removeAccumulatedInput(event.getContext());
+                                        final Filter filter = new Filter(name, predicate, packets);
 
                                         // Print the last line as well
                                         whom.sendRawMessage(prompt.getPromptText(event.getContext()));
@@ -412,7 +411,7 @@ public void conversationAbandoned(ConversationAbandonedEvent event) {
                                             // Force early compilation
                                             filter.compile(engine);
 
-                                            filters.add(filter);
+                                            filters.put(lowerCaseName, filter);
                                             whom.sendRawMessage(ChatColor.GOLD + "Added filter " + name);
                                         } catch (ScriptException e) {
                                             e.printStackTrace();
@@ -438,12 +437,12 @@ public void conversationAbandoned(ConversationAbandonedEvent event) {
                 break;
 
             case REMOVE:
-                Filter filter = findFilter(name);
+                final Filter filter = filters.get(lowerCaseName);
 
                 // See if it exists before we remove it
                 if (filter != null) {
                     filter.close(engine);
-                    filters.remove(filter);
+                    filters.remove(lowerCaseName);
                     sender.sendMessage(ChatColor.GOLD + "Removed filter " + name);
                 } else {
                     sender.sendMessage(ChatColor.RED + "Unable to find a filter by the name " + name);
@@ -453,22 +452,7 @@ public void conversationAbandoned(ConversationAbandonedEvent event) {
 
         return true;
     }
-    
-    /**
-     * Lookup a filter by its name.
-     * @param name - the filter name.
-     * @return The filter, or NULL if not found.
-     */
-    private Filter findFilter(String name) {
-        // We'll just use a linear scan for now - we don't expect that many filters
-        for (Filter filter : filters) {
-            if (filter.getName().equalsIgnoreCase(name)) {
-                return filter;
-            }
-        }
-        return null;
-    }
-    
+
     private SubCommand parseCommand(String[] args, int index) {
         String text = args[index].toUpperCase();
 

src/main/java/com/comphenix/protocol/CommandPacket.java

@@ -17,7 +17,21 @@
 
 package com.comphenix.protocol;
 
-import java.lang.reflect.InvocationTargetException;
+import com.comphenix.protocol.PacketType.Sender;
+import com.comphenix.protocol.concurrency.PacketTypeSet;
+import com.comphenix.protocol.error.ErrorReporter;
+import com.comphenix.protocol.error.ReportType;
+import com.comphenix.protocol.events.ListeningWhitelist;
+import com.comphenix.protocol.events.PacketEvent;
+import com.comphenix.protocol.events.PacketListener;
+import com.comphenix.protocol.utility.ChatExtensions;
+import com.comphenix.protocol.utility.HexDumper;
+import com.google.common.collect.MapMaker;
+import org.bukkit.ChatColor;
+import org.bukkit.command.CommandSender;
+import org.bukkit.entity.Player;
+import org.bukkit.plugin.Plugin;
+
 import java.util.ArrayDeque;
 import java.util.ArrayList;
 import java.util.Arrays;
@@ -30,23 +44,6 @@
 import java.util.logging.Level;
 import java.util.logging.Logger;
 
-import org.bukkit.ChatColor;
-import org.bukkit.command.CommandSender;
-import org.bukkit.entity.Player;
-import org.bukkit.plugin.Plugin;
-
-import com.comphenix.protocol.PacketType.Sender;
-import com.comphenix.protocol.concurrency.PacketTypeSet;
-import com.comphenix.protocol.error.ErrorReporter;
-import com.comphenix.protocol.error.Report;
-import com.comphenix.protocol.error.ReportType;
-import com.comphenix.protocol.events.ListeningWhitelist;
-import com.comphenix.protocol.events.PacketEvent;
-import com.comphenix.protocol.events.PacketListener;
-import com.comphenix.protocol.utility.ChatExtensions;
-import com.comphenix.protocol.utility.HexDumper;
-import com.google.common.collect.MapMaker;
-
 /**
  * Handles the "packet" debug command.
  * 
@@ -56,7 +53,7 @@ class CommandPacket extends CommandBase {
     public static final ReportType REPORT_CANNOT_SEND_MESSAGE = new ReportType("Cannot send chat message.");
 
     private enum SubCommand {
-        ADD, REMOVE, NAMES, PAGE;
+        ADD, REMOVE, NAMES, PAGE
     }
 
     /**
@@ -69,25 +66,25 @@ private enum SubCommand {
      */
     public static final int PAGE_LINE_COUNT = 9;
 
-    private Plugin plugin;
-    private Logger logger;
-    private ProtocolManager manager;
+    private final Plugin plugin;
+    private final Logger logger;
+    private final ProtocolManager manager;
 
-    private ChatExtensions chatter;
+    private final ChatExtensions chatter;
 
     // The main parser
-    private PacketTypeParser typeParser = new PacketTypeParser();
+    private final PacketTypeParser typeParser = new PacketTypeParser();
 
     // Paged message
-    private Map<CommandSender, List<String>> pagedMessage = new WeakHashMap<CommandSender, List<String>>();
+    private final Map<CommandSender, List<String>> pagedMessage = new WeakHashMap<CommandSender, List<String>>();
 
     // Current registered packet types
-    private PacketTypeSet packetTypes = new PacketTypeSet();
-    private PacketTypeSet extendedTypes = new PacketTypeSet();
+    private final PacketTypeSet packetTypes = new PacketTypeSet();
+    private final PacketTypeSet extendedTypes = new PacketTypeSet();
 
     // Compare listeners
-    private PacketTypeSet compareTypes = new PacketTypeSet();
-    private Map<PacketEvent, String> originalPackets = new MapMaker().weakKeys().makeMap();
+    private final PacketTypeSet compareTypes = new PacketTypeSet();
+    private final Map<PacketEvent, String> originalPackets = new MapMaker().weakKeys().makeMap();
 
     // The packet listener
     private PacketListener listener;
@@ -96,7 +93,7 @@ private enum SubCommand {
     private PacketListener compareListener;
 
     // Filter packet events
-    private CommandFilter filter;
+    private final CommandFilter filter;
 
     public CommandPacket(ErrorReporter reporter, Plugin plugin, Logger logger, CommandFilter filter, ProtocolManager manager) {
         super(reporter, CommandBase.PERMISSION_ADMIN, NAME, 1);
@@ -156,7 +153,7 @@ private void printPage(CommandSender sender, int pageIndex) {
     @Override
     protected boolean handleCommand(CommandSender sender, String[] args) {
         try {
-            Deque<String> arguments = new ArrayDeque<String>(Arrays.asList(args));
+            Deque<String> arguments = new ArrayDeque<>(Arrays.asList(args));
             SubCommand subCommand = parseCommand(arguments);
 
             // Commands with different parameters
@@ -247,14 +244,14 @@ private void executeRemoveCommand(CommandSender sender, Set<PacketType> removal)
     }
 
     private void executeNamesCommand(CommandSender sender, Set<PacketType> types) {
-        List<String> messages = new ArrayList<String>();
+        List<String> messages = new ArrayList<>();
 
         // Print the equivalent name of every given ID
         for (PacketType type : types) {
             messages.add(ChatColor.YELLOW + type.toString());
         }
 
-        if (sender instanceof Player && messages.size() > 0 && messages.size() > PAGE_LINE_COUNT) {
+        if (sender instanceof Player && messages.size() > PAGE_LINE_COUNT) {
             // Divide the messages into chuncks
             pagedMessage.put(sender, messages);
             printPage(sender, 1);
@@ -444,19 +441,6 @@ public PacketListener updatePacketListener() {
 
     private SubCommand parseCommand(Deque<String> arguments)
     {
-        final String text = arguments.remove().toLowerCase();
-
-        switch (text) {
-            case "add":
-                return SubCommand.ADD;
-            case "remove":
-                return SubCommand.REMOVE;
-            case "names":
-                return SubCommand.NAMES;
-            case "page":
-                return SubCommand.PAGE;
-            default:
-                throw new IllegalArgumentException(text + " is not a valid sub command. Must be add or remove.");
-        }
+        return SubCommand.valueOf(arguments.remove().toUpperCase());
     }
 }

src/main/java/com/comphenix/protocol/PacketLogging.java

@@ -158,7 +158,7 @@ public boolean onCommand(CommandSender sender, Command command, String label, St
             sender.sendMessage(ChatColor.RED + "Invalid syntax: /packetlog <protocol> <sender> <packet> [location]");
             return true;
         } catch (Throwable ex) {
-            sender.sendMessage(ChatColor.RED + "Failed to parse command: " + ex.toString());
+            sender.sendMessage(ChatColor.RED + "Failed to parse command: " + ex);
             return true;
         }
     }
@@ -173,7 +173,7 @@ private void startLogging() {
         this.sendingWhitelist = ListeningWhitelist.newBuilder().types(sendingTypes).build();
         this.receivingWhitelist = ListeningWhitelist.newBuilder().types(receivingTypes).build();
 
-        // Setup the file logger if it hasn't been already
+        // Set up the file logger if it hasn't been already
         if (location == LogLocation.FILE && fileLogger == null) {
             fileLogger = Logger.getLogger("ProtocolLib-FileLogging");
 
@@ -270,15 +270,8 @@ private static class LogFormatter extends Formatter {
 
         @Override
         public String format(LogRecord record) {
-            String string = formatMessage(record);
-            if (string.isEmpty()) {
-                return LINE_SEPARATOR;
-            }
-
-            StringBuilder message = new StringBuilder();
-            message.append(MessageFormat.format(FORMAT, DATE.format(record.getMillis()), string));
-            message.append(LINE_SEPARATOR);
-            return message.toString();
+            final String string = formatMessage(record);
+            return string.isEmpty() ? LINE_SEPARATOR : MessageFormat.format(FORMAT, DATE.format(record.getMillis()), string) + LINE_SEPARATOR;
         }
     }
 }