Library children object fix

tool/input_sdk/private/debugger.dart

 // Copyright (c) 2015, the Dart project authors.  Please see the AUTHORS file
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
 library dart._debugger;
 
 import 'dart:_foreign_helper' show JS;
 import 'dart:_runtime' as dart;
 import 'dart:core';
 import 'dart:collection';
@@ skipping 536 matching lines @@
 
   bool hasChildren(object) => true;
 
   String preview(object) => object.name;
 
   List<NameValuePair> children(object) {
     var children = new LinkedHashSet<NameValuePair>();
     var entry = object.object;
     for (var name in getOwnPropertyNames(entry)) {
       var value = safeGetProperty(entry, name);
       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.
 /// Dart functions happen to be regular JavaScript Function objects but
 /// we can distinguish them based on whether they have been tagged with
 /// runtime type information.
 class FunctionFormatter extends Formatter {
   accept(object) {
     if (_typeof(object) != 'function') return false;
     return dart.getReifiedType(object) != null;
@@ skipping 222 matching lines @@
 
   List<NameValuePair> children(object) => object.children();
 }
 
 /// This entry point is automatically invoked by the code generated by
 /// Dart Dev Compiler
 registerDevtoolsFormatter() {
   var formatters = [_devtoolsFormatter];
   JS('', 'dart.global.devtoolsFormatters = #', formatters);
 }