Review code of TypeDefinitionBuilder (#3064)

* Review code of TypeDefinitionBuilder 1. use init method to init builds' list * use single list for all builders. Seems like the builder is thread-safe, we can keep them static and final. * clean code.
apache · Jan 25, 2019 · ed8bb74 · ed8bb74
1 parent 26f010d
commit ed8bb74
Show file tree

Hide file tree

Showing 5 changed files with 17 additions and 43 deletions.
diff --git a/...-definition/src/main/java/org/apache/dubbo/metadata/definition/TypeDefinitionBuilder.java b/...-definition/src/main/java/org/apache/dubbo/metadata/definition/TypeDefinitionBuilder.java
@@ -26,6 +26,7 @@
 
 import java.lang.reflect.Type;
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
@@ -35,20 +36,16 @@
  */
 public class TypeDefinitionBuilder {
 
-    private static final ThreadLocal<ArrayList<TypeBuilder>> builders;
+    private static final TypeBuilder ARRAY_BUILDER = new ArrayTypeBuilder();
+    private static final TypeBuilder COLLECTION_BUILDER = new CollectionTypeBuilder();
+    private static final TypeBuilder MAP_BUILDER = new MapTypeBuilder();
+    private static final TypeBuilder ENUM_BUILDER = new EnumTypeBuilder();
 
-    static {
-        builders = new ThreadLocal<ArrayList<TypeBuilder>>();
-        builders.set(new ArrayList<TypeBuilder>());
-        builders.get().add(new ArrayTypeBuilder());
-        builders.get().add(new CollectionTypeBuilder());
-        builders.get().add(new MapTypeBuilder());
-        builders.get().add(new EnumTypeBuilder());
-    }
+    private static final List<TypeBuilder> BUILDERS = Arrays.asList(ARRAY_BUILDER, COLLECTION_BUILDER, MAP_BUILDER, ENUM_BUILDER);
 
     public static TypeDefinition build(Type type, Class<?> clazz, Map<Class<?>, TypeDefinition> typeCache) {
         TypeBuilder builder = getGenericTypeBuilder(type, clazz);
-        TypeDefinition td = null;
+        TypeDefinition td;
         if (builder != null) {
             td = builder.build(type, clazz, typeCache);
         } else {
@@ -58,22 +55,22 @@ public static TypeDefinition build(Type type, Class<?> clazz, Map<Class<?>, Type
     }
 
     private static TypeBuilder getGenericTypeBuilder(Type type, Class<?> clazz) {
-        for (TypeBuilder builder : builders.get()) {
+        for (TypeBuilder builder : BUILDERS) {
             if (builder.accept(type, clazz)) {
                 return builder;
             }
         }
         return null;
     }
 
-    private Map<Class<?>, TypeDefinition> typeCache = new HashMap<Class<?>, TypeDefinition>();
+    private Map<Class<?>, TypeDefinition> typeCache = new HashMap<>();
 
     public TypeDefinition build(Type type, Class<?> clazz) {
         return build(type, clazz, typeCache);
     }
 
     public List<TypeDefinition> getTypeDefinitions() {
-        return new ArrayList<TypeDefinition>(typeCache.values());
+        return new ArrayList<>(typeCache.values());
     }
 
 }
diff --git a/...finition/src/main/java/org/apache/dubbo/metadata/definition/builder/ArrayTypeBuilder.java b/...finition/src/main/java/org/apache/dubbo/metadata/definition/builder/ArrayTypeBuilder.java
@@ -32,12 +32,7 @@ public boolean accept(Type type, Class<?> clazz) {
         if (clazz == null) {
             return false;
         }
-
-        if (clazz.isArray()) {
-            return true;
-        }
-
-        return false;
+        return clazz.isArray();
     }
 
     @Override
@@ -47,8 +42,7 @@ public TypeDefinition build(Type type, Class<?> clazz, Map<Class<?>, TypeDefinit
         TypeDefinitionBuilder.build(componentType, componentType, typeCache);
 
         final String canonicalName = clazz.getCanonicalName();
-        TypeDefinition td = new TypeDefinition(canonicalName);
-        return td;
+        return new TypeDefinition(canonicalName);
     }
 
 }
diff --git a/...ion/src/main/java/org/apache/dubbo/metadata/definition/builder/CollectionTypeBuilder.java b/...ion/src/main/java/org/apache/dubbo/metadata/definition/builder/CollectionTypeBuilder.java
@@ -28,20 +28,14 @@
 /**
  * 2015/1/27.
  */
-public class
-CollectionTypeBuilder implements TypeBuilder {
+public class CollectionTypeBuilder implements TypeBuilder {
 
     @Override
     public boolean accept(Type type, Class<?> clazz) {
         if (clazz == null) {
             return false;
         }
-
-        if (Collection.class.isAssignableFrom(clazz)) {
-            return true;
-        }
-
-        return false;
+        return Collection.class.isAssignableFrom(clazz);
     }
 
     @Override
@@ -72,8 +66,7 @@ public TypeDefinition build(Type type, Class<?> clazz, Map<Class<?>, TypeDefinit
             }
         }
 
-        TypeDefinition td = new TypeDefinition(type.toString());
-        return td;
+        return new TypeDefinition(type.toString());
     }
 
 }
diff --git a/...efinition/src/main/java/org/apache/dubbo/metadata/definition/builder/EnumTypeBuilder.java b/...efinition/src/main/java/org/apache/dubbo/metadata/definition/builder/EnumTypeBuilder.java
@@ -32,12 +32,7 @@ public boolean accept(Type type, Class<?> clazz) {
         if (clazz == null) {
             return false;
         }
-
-        if (clazz.isEnum()) {
-            return true;
-        }
-
-        return false;
+        return clazz.isEnum();
     }
 
     @Override

diff --git a/...definition/src/main/java/org/apache/dubbo/metadata/definition/builder/MapTypeBuilder.java b/...definition/src/main/java/org/apache/dubbo/metadata/definition/builder/MapTypeBuilder.java
@@ -34,12 +34,7 @@ public boolean accept(Type type, Class<?> clazz) {
         if (clazz == null) {
             return false;
         }
-
-        if (Map.class.isAssignableFrom(clazz)) {
-            return true;
-        }
-
-        return false;
+        return Map.class.isAssignableFrom(clazz);
     }
 
     @Override