Library children object fix

tool/input_sdk/private/debugger.dart

Index: tool/input_sdk/private/debugger.dart
diff --git a/tool/input_sdk/private/debugger.dart b/tool/input_sdk/private/debugger.dart
index 8cdbeb0a45826ebc4dca27e7ad74b90edc36df5f..a701234e15060d0a91ca526ec99ede757ce555f1 100644
--- a/tool/input_sdk/private/debugger.dart
+++ b/tool/input_sdk/private/debugger.dart
@@ -557,33 +557,40 @@ class LibraryFormatter extends ObjectFormatter {
       if (value != null) {

[Alan Knight, 2016/07/25 20:53:00]

You could even split this up more/ reduce the level of nesting if you used
iterables. e.g.

safeProperties(entry) => getOwnPropertyNames(entry).map((name) =>
safeGetProperty(entry, name)).where((each) => each != null);

for (var value in safeProperties(entry)) {
 ....
}

There are some interesting questions, though. That works well for creating the
children, but you have the generic constructor issue, which has a side effect
and doesn't add to the list. And what happens if there's more than one generic
constructor in a class?

I might be tempted to split this into two loops, one for type parameters and one
for children. That's less efficient, but the number of children for any
non-collection type should be quite small, and I assume that for collections
we're already doing something to limit it.

[bmilligan, 2016/07/25 23:28:31]

The generic types that get caught in the the first part of the if statement do
not want to be added as children, only their parameters saved. So, I could
iterate through twice, but I would have to make the same check in both for that
generic type to make sure that in the "children" for loop, the generic types are
not being added. I changed how the generic parameters are saved, so that now
any/all generic types can be saved instead of just one.

         var genericTypeConstructor = dart.getGenericTypeCtor(value);
         if (genericTypeConstructor != null) {
-          genericName = name;
-          // Using JS toString() eliminates the leading metadata that is generated
-          // with the toString function provided in operations.dart.
-          // Splitting by => and taking the first element gives the list of
-          // arguments in the constructor.
-          genericArguments =
-              JS('String', '#.toString()', genericTypeConstructor)
-                  .split(' =>')
-                  .first
-                  .replaceAll(new RegExp(r'[(|)]'), '');
+          genericClassHandler(name, genericTypeConstructor);
         } else if (value is Type) {
-          var typeName = getTypeName(value);
-          // Generic class names are generated with a $ at the end, so the
-          // corresponding non-generic class can be identified by adding $.
-          if ('$name\$' == genericName) {
-            typeName = '$typeName<$genericArguments>';
-          }
-          children.add(new NameValuePair(
-              name: typeName, value: new ClassMetadata(value, name: typeName)));
+          addClassChild(name, value, children);

[Alan Knight, 2016/07/25 20:53:00]

Good, but maybe for consistency with the clause below, do it more like

   children.add(classChild(name, value))

[bmilligan, 2016/07/25 23:28:31]

Done.

         } else {
-          children.add(
-              new NameValuePair(name: name, value: new ClassMetadata(value)));
+          children.add(new NameValuePair(name: name, value: value));
         }
       }
     }
     return children.toList();
   }
+
+  genericClassHandler(String name, Object genericTypeConstructor) {

[Alan Knight, 2016/07/25 20:53:00]

"Handler" is another one of those non-value-adding words. Also, better to name
methods as verbs than nouns. Better to call this something like noteGenerics or
recordTypeParameters

[bmilligan, 2016/07/25 23:28:31]

Done.

+    genericName = name;
+    // Using JS toString() eliminates the leading metadata that is generated
+    // with the toString function provided in operations.dart.
+    // Splitting by => and taking the first element gives the list of
+    // arguments in the constructor.
+    genericArguments = JS('String', '#.toString()', genericTypeConstructor)
+        .split(' =>')
+        .first
+        .replaceAll(new RegExp(r'[(|)]'), '');
+  }
+
+  addClassChild(
+      String name, Object child, LinkedHashSet<NameValuePair> children) {
+    var typeName = getTypeName(child);
+    // Generic class names are generated with a $ at the end, so the
+    // corresponding non-generic class can be identified by adding $.
+    if ('$name\$' == genericName) {
+      typeName = '$typeName<$genericArguments>';
+    }
+    children.add(new NameValuePair(
+        name: typeName, value: new ClassMetadata(child, name: typeName)));
+  }
 }
 
 /// Formatter for Dart Function objects.