From c26b361b3605d3863dc2f016904ef531e55d560d Mon Sep 17 00:00:00 2001 From: xiuyuhang <442367943@qq.com> Date: Wed, 26 Dec 2018 17:32:57 +0800 Subject: [PATCH 1/3] Review code of TypeDefinitionBuilder 1. use init method to init builds' list --- .../definition/TypeDefinitionBuilder.java | 24 +++++++++---------- 1 file changed, 11 insertions(+), 13 deletions(-) diff --git a/dubbo-metadata-report/dubbo-metadata-definition/src/main/java/org/apache/dubbo/metadata/definition/TypeDefinitionBuilder.java b/dubbo-metadata-report/dubbo-metadata-definition/src/main/java/org/apache/dubbo/metadata/definition/TypeDefinitionBuilder.java index 797d9f8a0e5..12c73e02a87 100755 --- a/dubbo-metadata-report/dubbo-metadata-definition/src/main/java/org/apache/dubbo/metadata/definition/TypeDefinitionBuilder.java +++ b/dubbo-metadata-report/dubbo-metadata-definition/src/main/java/org/apache/dubbo/metadata/definition/TypeDefinitionBuilder.java @@ -35,20 +35,18 @@ */ public class TypeDefinitionBuilder { - private static final ThreadLocal> builders; - - static { - builders = new ThreadLocal>(); - builders.set(new ArrayList()); - builders.get().add(new ArrayTypeBuilder()); - builders.get().add(new CollectionTypeBuilder()); - builders.get().add(new MapTypeBuilder()); - builders.get().add(new EnumTypeBuilder()); - } + private static final ThreadLocal> builders = ThreadLocal.withInitial(() -> { + ArrayList l = new ArrayList<>(); + l.add(new ArrayTypeBuilder()); + l.add(new CollectionTypeBuilder()); + l.add(new MapTypeBuilder()); + l.add(new EnumTypeBuilder()); + return l; + }); public static TypeDefinition build(Type type, Class clazz, Map, TypeDefinition> typeCache) { TypeBuilder builder = getGenericTypeBuilder(type, clazz); - TypeDefinition td = null; + TypeDefinition td; if (builder != null) { td = builder.build(type, clazz, typeCache); } else { @@ -66,14 +64,14 @@ private static TypeBuilder getGenericTypeBuilder(Type type, Class clazz) { return null; } - private Map, TypeDefinition> typeCache = new HashMap, TypeDefinition>(); + private Map, TypeDefinition> typeCache = new HashMap<>(); public TypeDefinition build(Type type, Class clazz) { return build(type, clazz, typeCache); } public List getTypeDefinitions() { - return new ArrayList(typeCache.values()); + return new ArrayList<>(typeCache.values()); } } From cfd5cd5fa3352ca0df3cc6e18ed4e5b706a7852a Mon Sep 17 00:00:00 2001 From: xiuyuhang <442367943@qq.com> Date: Fri, 28 Dec 2018 14:07:39 +0800 Subject: [PATCH 2/3] use single list for all builders. Seems like the builder is thread-safe, we can keep them static and final. --- .../definition/TypeDefinitionBuilder.java | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/dubbo-metadata-report/dubbo-metadata-definition/src/main/java/org/apache/dubbo/metadata/definition/TypeDefinitionBuilder.java b/dubbo-metadata-report/dubbo-metadata-definition/src/main/java/org/apache/dubbo/metadata/definition/TypeDefinitionBuilder.java index 12c73e02a87..53e8c88b8f3 100755 --- a/dubbo-metadata-report/dubbo-metadata-definition/src/main/java/org/apache/dubbo/metadata/definition/TypeDefinitionBuilder.java +++ b/dubbo-metadata-report/dubbo-metadata-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,14 +36,12 @@ */ public class TypeDefinitionBuilder { - private static final ThreadLocal> builders = ThreadLocal.withInitial(() -> { - ArrayList l = new ArrayList<>(); - l.add(new ArrayTypeBuilder()); - l.add(new CollectionTypeBuilder()); - l.add(new MapTypeBuilder()); - l.add(new EnumTypeBuilder()); - return l; - }); + 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(); + + private static final List BUILDERS = Arrays.asList(ARRAY_BUILDER, COLLECTION_BUILDER, MAP_BUILDER, ENUM_BUILDER); public static TypeDefinition build(Type type, Class clazz, Map, TypeDefinition> typeCache) { TypeBuilder builder = getGenericTypeBuilder(type, clazz); @@ -56,7 +55,7 @@ public static TypeDefinition build(Type type, Class clazz, Map, 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; } From e9cd3654aec95c794738aa141c4b20cfe5a709bb Mon Sep 17 00:00:00 2001 From: xiuyuhang <442367943@qq.com> Date: Fri, 28 Dec 2018 14:10:48 +0800 Subject: [PATCH 3/3] clean code. --- .../definition/builder/ArrayTypeBuilder.java | 10 ++-------- .../definition/builder/CollectionTypeBuilder.java | 13 +++---------- .../definition/builder/EnumTypeBuilder.java | 7 +------ .../metadata/definition/builder/MapTypeBuilder.java | 7 +------ 4 files changed, 7 insertions(+), 30 deletions(-) diff --git a/dubbo-metadata-report/dubbo-metadata-definition/src/main/java/org/apache/dubbo/metadata/definition/builder/ArrayTypeBuilder.java b/dubbo-metadata-report/dubbo-metadata-definition/src/main/java/org/apache/dubbo/metadata/definition/builder/ArrayTypeBuilder.java index ce53c708682..9ad61cefb26 100755 --- a/dubbo-metadata-report/dubbo-metadata-definition/src/main/java/org/apache/dubbo/metadata/definition/builder/ArrayTypeBuilder.java +++ b/dubbo-metadata-report/dubbo-metadata-definition/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, 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/dubbo-metadata-report/dubbo-metadata-definition/src/main/java/org/apache/dubbo/metadata/definition/builder/CollectionTypeBuilder.java b/dubbo-metadata-report/dubbo-metadata-definition/src/main/java/org/apache/dubbo/metadata/definition/builder/CollectionTypeBuilder.java index 14c3b4ba68c..24b1c868a1b 100755 --- a/dubbo-metadata-report/dubbo-metadata-definition/src/main/java/org/apache/dubbo/metadata/definition/builder/CollectionTypeBuilder.java +++ b/dubbo-metadata-report/dubbo-metadata-definition/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, TypeDefinit } } - TypeDefinition td = new TypeDefinition(type.toString()); - return td; + return new TypeDefinition(type.toString()); } } diff --git a/dubbo-metadata-report/dubbo-metadata-definition/src/main/java/org/apache/dubbo/metadata/definition/builder/EnumTypeBuilder.java b/dubbo-metadata-report/dubbo-metadata-definition/src/main/java/org/apache/dubbo/metadata/definition/builder/EnumTypeBuilder.java index cca8c9ca81d..d0dea96302c 100755 --- a/dubbo-metadata-report/dubbo-metadata-definition/src/main/java/org/apache/dubbo/metadata/definition/builder/EnumTypeBuilder.java +++ b/dubbo-metadata-report/dubbo-metadata-definition/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/dubbo-metadata-report/dubbo-metadata-definition/src/main/java/org/apache/dubbo/metadata/definition/builder/MapTypeBuilder.java b/dubbo-metadata-report/dubbo-metadata-definition/src/main/java/org/apache/dubbo/metadata/definition/builder/MapTypeBuilder.java index 4c7cf93c866..bf2b22209cd 100755 --- a/dubbo-metadata-report/dubbo-metadata-definition/src/main/java/org/apache/dubbo/metadata/definition/builder/MapTypeBuilder.java +++ b/dubbo-metadata-report/dubbo-metadata-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