From af0d41fe67efb0a8aee67de5f7dd942e3e88def4 Mon Sep 17 00:00:00 2001 From: Robert Varga Date: Thu, 1 Mar 2018 16:48:53 +0100 Subject: [PATCH] Optimize generated toString() Rather than inlining the equivalent of a ToStringHelper, add utility methods to CodeHelpers and use ToStringHelper to generate the string. Change-Id: If8c8d03d90715e87d25262056c2e6db544e091bb Signed-off-by: Robert Varga --- .../java/api/generator/BaseTemplate.xtend | 23 +--- .../java/api/generator/BuilderTemplate.xtend | 37 ++----- .../api/generator/BuilderGeneratorTest.java | 100 ++++++++++-------- .../yangtools/yang/binding/CodeHelpers.java | 30 ++++++ 4 files changed, 96 insertions(+), 94 deletions(-) diff --git a/binding/mdsal-binding-java-api-generator/src/main/java/org/opendaylight/mdsal/binding/java/api/generator/BaseTemplate.xtend b/binding/mdsal-binding-java-api-generator/src/main/java/org/opendaylight/mdsal/binding/java/api/generator/BaseTemplate.xtend index c02a2d2960..ba43a0abde 100644 --- a/binding/mdsal-binding-java-api-generator/src/main/java/org/opendaylight/mdsal/binding/java/api/generator/BaseTemplate.xtend +++ b/binding/mdsal-binding-java-api-generator/src/main/java/org/opendaylight/mdsal/binding/java/api/generator/BaseTemplate.xtend @@ -10,9 +10,9 @@ package org.opendaylight.mdsal.binding.java.api.generator import static org.opendaylight.mdsal.binding.model.util.BindingGeneratorUtil.encodeAngleBrackets import com.google.common.base.CharMatcher +import com.google.common.base.MoreObjects import com.google.common.base.Splitter import com.google.common.collect.Iterables -import java.util.Arrays import java.util.Collection import java.util.List import java.util.StringTokenizer @@ -25,6 +25,7 @@ import org.opendaylight.mdsal.binding.model.api.TypeMember import org.opendaylight.mdsal.binding.model.api.YangSourceDefinition.Single import org.opendaylight.mdsal.binding.model.api.YangSourceDefinition.Multiple import org.opendaylight.mdsal.binding.model.util.Types +import org.opendaylight.yangtools.yang.binding.CodeHelpers import org.opendaylight.yangtools.yang.common.QName import org.opendaylight.yangtools.yang.model.api.ContainerSchemaNode import org.opendaylight.yangtools.yang.model.api.DocumentedNode @@ -382,25 +383,11 @@ abstract class BaseTemplate extends JavaFileTemplate { «IF !properties.empty» @Override public «String.importedName» toString() { - «StringBuilder.importedName» builder = new «StringBuilder.importedName»(«type.importedName».class.getSimpleName()).append(" ["); - boolean first = true; - + final «MoreObjects.importedName».ToStringHelper helper = «MoreObjects.importedName».toStringHelper(«type.importedName».class); «FOR property : properties» - if («property.fieldName» != null) { - if (first) { - first = false; - } else { - builder.append(", "); - } - builder.append("«property.fieldName»="); - «IF property.returnType.name.contains("[")» - builder.append(«Arrays.importedName».toString(«property.fieldName»)); - «ELSE» - builder.append(«property.fieldName»); - «ENDIF» - } + «CodeHelpers.importedName».appendValue(helper, "«property.fieldName»", «property.fieldName»); «ENDFOR» - return builder.append(']').toString(); + return helper.toString(); } «ENDIF» ''' diff --git a/binding/mdsal-binding-java-api-generator/src/main/java/org/opendaylight/mdsal/binding/java/api/generator/BuilderTemplate.xtend b/binding/mdsal-binding-java-api-generator/src/main/java/org/opendaylight/mdsal/binding/java/api/generator/BuilderTemplate.xtend index a9cabecd83..0e231a0945 100644 --- a/binding/mdsal-binding-java-api-generator/src/main/java/org/opendaylight/mdsal/binding/java/api/generator/BuilderTemplate.xtend +++ b/binding/mdsal-binding-java-api-generator/src/main/java/org/opendaylight/mdsal/binding/java/api/generator/BuilderTemplate.xtend @@ -7,6 +7,7 @@ */ package org.opendaylight.mdsal.binding.java.api.generator +import com.google.common.base.MoreObjects import com.google.common.collect.ImmutableMap import com.google.common.collect.ImmutableSortedSet import java.util.ArrayList @@ -32,9 +33,9 @@ import org.opendaylight.mdsal.binding.model.util.generated.type.builder.Generate import org.opendaylight.yangtools.concepts.Builder import org.opendaylight.yangtools.yang.binding.Augmentable import org.opendaylight.yangtools.yang.binding.AugmentationHolder +import org.opendaylight.yangtools.yang.binding.CodeHelpers import org.opendaylight.yangtools.yang.binding.DataObject import org.opendaylight.yangtools.yang.binding.Identifiable -import org.opendaylight.yangtools.yang.binding.CodeHelpers /** * Template for generating JAVA builder classes. @@ -697,39 +698,17 @@ class BuilderTemplate extends BaseTemplate { ''' def override generateToString(Collection properties) ''' - «IF !(properties === null)» + «IF properties !== null» @Override public «String.importedName» toString() { - «String.importedName» name = "«type.name» ["; - «StringBuilder.importedName» builder = new «StringBuilder.importedName» (name); - «FOR property : properties SEPARATOR "\n builder.append(\", \");\n}" AFTER " }\n"» - if («property.fieldName» != null) { - builder.append("«property.fieldName»="); - «IF property.returnType.name.contains("[")» - builder.append(«Arrays.importedName».toString(«property.fieldName»)); - «ELSE» - builder.append(«property.fieldName»); - «ENDIF» + final «MoreObjects.importedName».ToStringHelper helper = «MoreObjects.importedName».toStringHelper("«type.name»"); + «FOR property : properties» + «CodeHelpers.importedName».appendValue(helper, "«property.fieldName»", «property.fieldName»); «ENDFOR» «IF augmentField !== null» - «IF !properties.empty» - «««Append comma separator only if it's not there already from previous operation»»» -final int builderLength = builder.length(); - final int builderAdditionalLength = builder.substring(name.length(), builderLength).length(); - if (builderAdditionalLength > 2 && !builder.substring(builderLength - 2, builderLength).equals(", ")) { - builder.append(", "); - } - «ENDIF» - builder.append("«augmentField.name»="); - builder.append(«augmentField.name».values());«"\n"» - return builder.append(']').toString(); - «ELSE» - «IF properties.empty» - return builder.append(']').toString(); - «ELSE» - return builder.append(']').toString(); - «ENDIF» + «CodeHelpers.importedName».appendValue(helper, "«augmentField.name»", «augmentField.name».values()); «ENDIF» + return helper.toString(); } «ENDIF» ''' diff --git a/binding/mdsal-binding-java-api-generator/src/test/java/org/opendaylight/mdsal/binding/java/api/generator/BuilderGeneratorTest.java b/binding/mdsal-binding-java-api-generator/src/test/java/org/opendaylight/mdsal/binding/java/api/generator/BuilderGeneratorTest.java index c5959df04c..2fa5bc7971 100644 --- a/binding/mdsal-binding-java-api-generator/src/test/java/org/opendaylight/mdsal/binding/java/api/generator/BuilderGeneratorTest.java +++ b/binding/mdsal-binding-java-api-generator/src/test/java/org/opendaylight/mdsal/binding/java/api/generator/BuilderGeneratorTest.java @@ -39,79 +39,82 @@ public class BuilderGeneratorTest { private static final String TEST = "test"; @Test - public void basicTest() throws Exception { + public void basicTest() { assertEquals("", new BuilderGenerator().generate(mock(Type.class))); } @Test - public void builderTemplateGenerateToStringWithPropertyTest() throws Exception { + public void builderTemplateGenerateToStringWithPropertyTest() { final GeneratedType genType = mockGenType("get" + TEST); - final String generateToString = genToString(genType).toString(); - final String expected = GEN_TO_STRING_FIRST_PART - + "\n if (_test != null) {\n builder.append(\"_test=\");\n builder.append(_test);\n }" - + GEN_TO_STRING_LAST_PART; - assertEquals(expected, generateToString); + + assertEquals("@Override\n" + + "public java.lang.String toString() {\n" + + " final MoreObjects.ToStringHelper helper = MoreObjects.toStringHelper(\"test\");\n" + + " CodeHelpers.appendValue(helper, \"_test\", _test);\n" + + " return helper.toString();\n" + + "}\n", genToString(genType).toString()); } @Test public void builderTemplateGenerateToStringWithoutAnyPropertyTest() throws Exception { - final GeneratedType genType = mockGenType(TEST); - final String generateToString = genToString(genType).toString(); - final String expected = GEN_TO_STRING_FIRST_PART + GEN_TO_STRING_LAST_PART; - assertEquals(expected, generateToString); + assertEquals("@Override\n" + + "public java.lang.String toString() {\n" + + " final MoreObjects.ToStringHelper helper = MoreObjects.toStringHelper(\"test\");\n" + + " return helper.toString();\n" + + "}\n", genToString(mockGenType(TEST)).toString()); } @Test public void builderTemplateGenerateToStringWithMorePropertiesTest() throws Exception { - final GeneratedType genType = mockGenTypeMoreMeth("get" + TEST); - final String generateToString = genToString(genType).toString(); - final String expected = GEN_TO_STRING_FIRST_PART - + "\n if (_test1 != null) {\n builder.append(\"_test1=\");\n builder.append(_test1);" - + "\n " + APPEND_COMMA + "\n }" - + "\n if (_test2 != null) {\n builder.append(\"_test2=\");\n builder.append(_test2);\n }" - + GEN_TO_STRING_LAST_PART; - assertEquals(expected, generateToString); + assertEquals("@Override\n" + + "public java.lang.String toString() {\n" + + " final MoreObjects.ToStringHelper helper = MoreObjects.toStringHelper(\"test\");\n" + + " CodeHelpers.appendValue(helper, \"_test1\", _test1);\n" + + " CodeHelpers.appendValue(helper, \"_test2\", _test2);\n" + + " return helper.toString();\n" + + "}\n", genToString(mockGenTypeMoreMeth("get" + TEST)).toString()); } @Test public void builderTemplateGenerateToStringWithoutPropertyWithAugmentTest() throws Exception { - final GeneratedType genType = mockGenType(TEST); - mockAugment(genType); - final String generateToString = genToString(genType).toString(); - final String expected = GEN_TO_STRING_FIRST_PART + GEN_TO_STRING_AUGMENT_PART + GEN_TO_STRING_LAST_PART; - assertEquals(expected, generateToString); + assertEquals("@Override\n" + + "public java.lang.String toString() {\n" + + " final MoreObjects.ToStringHelper helper = MoreObjects.toStringHelper(\"test\");\n" + + " CodeHelpers.appendValue(helper, \"augmentation\", augmentation.values()); \n" + + " return helper.toString();\n" + + "}\n", genToString(mockAugment(mockGenType(TEST))).toString()); } @Test public void builderTemplateGenerateToStringWithPropertyWithAugmentTest() throws Exception { - final GeneratedType genType = mockGenType("get" + TEST); - mockAugment(genType); - final String generateToString = genToString(genType).toString(); - final String expected = GEN_TO_STRING_FIRST_PART - + "\n if (_test != null) {\n builder.append(\"_test=\");\n builder.append(_test);\n }" - + "\n " + APPEND_COMMA_AUGMENT + GEN_TO_STRING_AUGMENT_PART + GEN_TO_STRING_LAST_PART; - assertEquals(expected, generateToString); + assertEquals("@Override\n" + + "public java.lang.String toString() {\n" + + " final MoreObjects.ToStringHelper helper = MoreObjects.toStringHelper(\"test\");\n" + + " CodeHelpers.appendValue(helper, \"_test\", _test);\n" + + " CodeHelpers.appendValue(helper, \"augmentation\", augmentation.values()); \n" + + " return helper.toString();\n" + + "}\n", genToString(mockAugment(mockGenType("get" + TEST))).toString()); } @Test public void builderTemplateGenerateToStringWithMorePropertiesWithAugmentTest() throws Exception { - final GeneratedType genType = mockGenTypeMoreMeth("get" + TEST); - mockAugment(genType); - final String generateToString = genToString(genType).toString(); - final String expected = GEN_TO_STRING_FIRST_PART - + "\n if (_test1 != null) {\n builder.append(\"_test1=\");\n builder.append(_test1);\n " - + APPEND_COMMA + "\n }" - + "\n if (_test2 != null) {\n builder.append(\"_test2=\");\n builder.append(_test2);\n }" - + "\n " + APPEND_COMMA_AUGMENT + GEN_TO_STRING_AUGMENT_PART + GEN_TO_STRING_LAST_PART; - assertEquals(expected, generateToString); + assertEquals("@Override\n" + + "public java.lang.String toString() {\n" + + " final MoreObjects.ToStringHelper helper = MoreObjects.toStringHelper(\"test\");\n" + + " CodeHelpers.appendValue(helper, \"_test1\", _test1);\n" + + " CodeHelpers.appendValue(helper, \"_test2\", _test2);\n" + + " CodeHelpers.appendValue(helper, \"augmentation\", augmentation.values()); \n" + + " return helper.toString();\n" + + "}\n", genToString(mockAugment(mockGenTypeMoreMeth("get" + TEST))).toString()); } - private static void mockAugment(final GeneratedType genType) { + private static GeneratedType mockAugment(final GeneratedType genType) { final List impls = new ArrayList<>(); final Type impl = mock(Type.class); doReturn("org.opendaylight.yangtools.yang.binding.Augmentable").when(impl).getFullyQualifiedName(); impls.add(impl); doReturn(impls).when(genType).getImplements(); + return genType; } private static GeneratedType mockGenTypeMoreMeth(final String methodeName) { @@ -132,12 +135,15 @@ public class BuilderGeneratorTest { } @SuppressWarnings("unchecked") - private static CharSequence genToString(final GeneratedType genType) - throws NoSuchFieldException, IllegalAccessException { - final BuilderTemplate bt = new BuilderTemplate(genType); - final Field propertiesField = bt.getClass().getDeclaredField(PROPERTIES_FIELD_NAME); - propertiesField.setAccessible(true); - return bt.generateToString((Collection) propertiesField.get(bt)); + private static CharSequence genToString(final GeneratedType genType) { + try { + final BuilderTemplate bt = new BuilderTemplate(genType); + final Field propertiesField = bt.getClass().getDeclaredField(PROPERTIES_FIELD_NAME); + propertiesField.setAccessible(true); + return bt.generateToString((Collection) propertiesField.get(bt)); + } catch (IllegalArgumentException | IllegalAccessException | NoSuchFieldException | SecurityException e) { + throw new RuntimeException(e); + } } private static GeneratedType mockGenType(final String methodeName) { diff --git a/binding/yang-binding/src/main/java/org/opendaylight/yangtools/yang/binding/CodeHelpers.java b/binding/yang-binding/src/main/java/org/opendaylight/yangtools/yang/binding/CodeHelpers.java index 068de8a91e..f8b529ae64 100644 --- a/binding/yang-binding/src/main/java/org/opendaylight/yangtools/yang/binding/CodeHelpers.java +++ b/binding/yang-binding/src/main/java/org/opendaylight/yangtools/yang/binding/CodeHelpers.java @@ -11,6 +11,7 @@ import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.base.Verify.verify; import static java.util.Objects.requireNonNull; +import com.google.common.base.MoreObjects.ToStringHelper; import com.google.common.base.VerifyException; import java.util.Arrays; import java.util.List; @@ -64,6 +65,35 @@ public final class CodeHelpers { return value; } + /** + * Append a named value to a ToStringHelper. If the value is null, this method does nothing. + * + * @param helper Helper to append to + * @param name Name of the value + * @param value Value to append + * @throws NullPointerException if the name or helper is null + */ + public static void appendValue(final @NonNull ToStringHelper helper, final @NonNull String name, + final @Nullable Object value) { + if (value != null) { + helper.add(name, value); + } + } + + /** + * Append a named value to a ToStringHelper. If the value is null, this method does nothing. + * + * @param helper Helper to append to + * @param name Name of the value + * @param value Value to append + * @throws NullPointerException if the name or helper is null + */ + public static void appendValue(final ToStringHelper helper, final String name, final byte[] value) { + if (value != null) { + helper.add(name, Arrays.toString(value)); + } + } + /** * Compile a list of pattern regular expressions and return them as an array. The list must hold at least two * expressions. -- 2.36.6