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 <robert.varga@pantheon.tech>

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 c02a2d29609d225fbece5b12d02096474d7a0c62..ba43a0abde10289668164abc2e99b3209e4b7b7f 100644 (file)
--- 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 a9cabecd83214bb3a0710b98ba0b4609e482afbd..0e231a09454d9cb1a49d02b2560d52c8757cfbad 100644 (file)
--- 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<GeneratedProperty> 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 c5959df04c46554e68c85cc4b4bcea07251db500..2fa5bc79711c200cca47104577412542d605603d 100644 (file)
--- 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<Type> 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<GeneratedProperty>) 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<GeneratedProperty>) 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 068de8a91eee686a9e6eca242dc3d0edb35e291d..f8b529ae64acf4f7a98465e2759a6d1ceb17403f 100644 (file)
--- 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.