Cache material data constructors. Fixes BUKKIT-2980

Reobtaining a constructor is not a trivial operation, this change makes the Material enum store the respective constructors for each MaterialData.

Additionally 'fixed' the material tests to use proper generics.

2 files changed, 27 insertions, 32 deletions

diff --git a/src/main/java/org/bukkit/Material.java b/src/main/java/org/bukkit/Material.java
index f9ecad81..16cbe698 100644
--- a/src/main/java/org/bukkit/Material.java
+++ b/src/main/java/org/bukkit/Material.java
@@ -1,10 +1,7 @@
 package org.bukkit;
 
 import java.lang.reflect.Constructor;
-import java.lang.reflect.InvocationTargetException;
 import java.util.Map;
-import java.util.logging.Level;
-import java.util.logging.Logger;
 
 import org.apache.commons.lang.Validate;
 import org.bukkit.map.MapView;
@@ -331,7 +328,7 @@ public enum Material {
     ;
 
     private final int id;
-    private final Class<? extends MaterialData> data;
+    private final Constructor<? extends MaterialData> ctor;
     private static Material[] byId = new Material[383];
     private final static Map<String, Material> BY_NAME = Maps.newHashMap();
     private final int maxStack;
@@ -342,11 +339,11 @@ public enum Material {
     }
 
     private Material(final int id, final int stack) {
-        this(id, stack, null);
+        this(id, stack, MaterialData.class);
     }
 
     private Material(final int id, final int stack, final int durability) {
-        this(id, stack, durability, null);
+        this(id, stack, durability, MaterialData.class);
     }
 
     private Material(final int id, final Class<? extends MaterialData> data) {
@@ -361,7 +358,14 @@ public enum Material {
         this.id = id;
         this.durability = (short) durability;
         this.maxStack = stack;
-        this.data = data == null ? MaterialData.class : data;
+        // try to cache the constructor for this material
+        try {
+            this.ctor = data.getConstructor(int.class, byte.class);
+        } catch (NoSuchMethodException ex) {
+            throw new AssertionError(ex);
+        } catch (SecurityException ex) {
+            throw new AssertionError(ex);
+        }
     }
 
     /**
@@ -397,7 +401,7 @@ public enum Material {
      * @return MaterialData associated with this Material
      */
     public Class<? extends MaterialData> getData() {
-        return data;
+        return ctor.getDeclaringClass();
     }
 
     /**
@@ -409,24 +413,19 @@ public enum Material {
      */
     public MaterialData getNewData(final byte raw) {
         try {
-            Constructor<? extends MaterialData> ctor = data.getConstructor(int.class, byte.class);
-
             return ctor.newInstance(id, raw);
         } catch (InstantiationException ex) {
-            Logger.getLogger(Material.class.getName()).log(Level.SEVERE, null, ex);
-        } catch (IllegalAccessException ex) {
-            Logger.getLogger(Material.class.getName()).log(Level.SEVERE, null, ex);
-        } catch (IllegalArgumentException ex) {
-            Logger.getLogger(Material.class.getName()).log(Level.SEVERE, null, ex);
-        } catch (InvocationTargetException ex) {
-            Logger.getLogger(Material.class.getName()).log(Level.SEVERE, null, ex);
-        } catch (NoSuchMethodException ex) {
-            Logger.getLogger(Material.class.getName()).log(Level.SEVERE, null, ex);
-        } catch (SecurityException ex) {
-            Logger.getLogger(Material.class.getName()).log(Level.SEVERE, null, ex);
+            final Throwable t = ex.getCause();
+            if (t instanceof RuntimeException) {
+                throw (RuntimeException) t;
+            }
+            if (t instanceof Error) {
+                throw (Error) t;
+            }
+            throw new AssertionError(t);
+        } catch (Throwable t) {
+            throw new AssertionError(t);
         }
-
-        return null;
     }
 
     /**
diff --git a/src/test/java/org/bukkit/MaterialTest.java b/src/test/java/org/bukkit/MaterialTest.java
index 46be920b..c1051caf 100644
--- a/src/test/java/org/bukkit/MaterialTest.java
+++ b/src/test/java/org/bukkit/MaterialTest.java
@@ -1,11 +1,9 @@
 package org.bukkit;
 
-import static org.hamcrest.CoreMatchers.is;
-import static org.hamcrest.CoreMatchers.isA;
-import static org.hamcrest.CoreMatchers.nullValue;
-import static org.junit.Assert.assertThat;
-import static org.junit.Assert.assertTrue;
+import static org.hamcrest.Matchers.*;
+import static org.junit.Assert.*;
 
+import org.bukkit.material.MaterialData;
 import org.junit.Test;
 
 public class MaterialTest {
@@ -42,14 +40,12 @@ public class MaterialTest {
         assertThat(Material.getMaterial(null), is(nullValue()));
     }
 
-    // [EB]: gawd we need better code >.>
-    @SuppressWarnings({ "unchecked", "rawtypes" })
     @Test
     public void getData() {
         for (Material material : Material.values()) {
-            Class clazz = material.getData();
+            Class<? extends MaterialData> clazz = material.getData();
 
-            assertThat(material.getNewData((byte) 0), isA(clazz));
+            assertThat(material.getNewData((byte) 0), is(instanceOf(clazz)));
         }
     }