Reinstall previous behavior for Set and Queue toString.

sdk/lib/collection/iterable.dart

 // Copyright (c) 2012, 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.
 
 part of dart.collection;
 
 /**
  * This [Iterable] mixin implements all [Iterable] members except `iterator`.
  *
  * All other methods are implemented in terms of `iterator`.
@@ skipping 177 matching lines @@
   E elementAt(int index) {
     if (index is! int || index < 0) throw new RangeError.value(index);
     int remaining = index;
     for (E element in this) {
       if (remaining == 0) return element;
       remaining--;
     }
     throw new RangeError.value(index);
   }
 
-  String toString() => _iterableToString(this);
+  String toString() => IterableBase.iterableToShortString(this, '(', ')');
 }
 
 /**
  * Base class for implementing [Iterable].
  *
  * This class implements all methods of [Iterable] except [Iterable.iterator]
  * in terms of `iterator`.
  */
 abstract class IterableBase<E> implements Iterable<E> {
   // TODO(lrn): Base this on IterableMixin if there ever becomes a way
@@ skipping 186 matching lines @@
    * Elements are represented by their own `toString` results.
    *
    * The representation always contains the first three elements.
    * If there are less than a hundred elements in the iterable, it also
    * contains the last two elements.
    *
    * If the resulting string isn't above 80 characters, more elements are
    * included from the start of the iterable.
    *
    * The conversion may omit calling `toString` on some elements if they
-   * are known to now occur in the output, and it may stop iterating after
+   * are known to not occur in the output, and it may stop iterating after
    * a hundred elements.
    */
-  String toString() => _iterableToString(this);
-}
+  String toString() => IterableBase.iterableToShortString(this, '(', ')');

[floitsch, 2014/05/23 16:09:58]

-IterableBase.-

[Lasse Reichstein Nielsen, 2014/05/23 16:50:01]

You mean I should remove it?
It is in the same class now, so I guess it's safe.

 
-String _setToString(Set set) => _collectionToString(set, "{" , "}");
-
-String _iterableToString(Iterable iterable) =>
-    _collectionToString(iterable, "(", ")");
-
-String _collectionToString(Iterable iterable, String before, String after) {
-  if (_toStringVisiting.contains(iterable)) return "$before...$after";
-  _toStringVisiting.add(iterable);
-  List parts = [];
-  try {
-    _collectionPartsToStrings(iterable, parts);
-  } finally {
-    _toStringVisiting.remove(iterable);
-  }
-  return (new StringBuffer(before)
-              ..writeAll(parts, ", ")
-              ..write(after)).toString();
-}
-
-/** Convert elments of [iterable] to strings and store them in [parts]. */
-void _collectionPartsToStrings(Iterable iterable, List parts) {
-  /// Try to stay below this many characters.
-  const int LENGTH_LIMIT = 80;
-  /// Always at least this many elements at the start.
-  const int HEAD_COUNT = 3;
-  /// Always at least this many elements at the end.
-  const int TAIL_COUNT = 2;
-  /// Stop iterating after this many elements. Iterables can be infinite.
-  const int MAX_COUNT = 100;
-  // Per entry length overhead. It's for ", " for all after the first entry,
-  // and for "(" and ")" for the initial entry. By pure luck, that's the same
-  // number.
-  const int OVERHEAD = 2;
-  const int ELLIPSIS_SIZE = 3;  // "...".length.
-  int length = 0;
-  int count = 0;
-  Iterator it = iterable.iterator;
-  // Initial run of elements, at least HEAD_COUNT, and then continue until
-  // passing at most LENGTH_LIMIT characters.
-  while (length < LENGTH_LIMIT || count < HEAD_COUNT) {
-    if (!it.moveNext()) {
-      return;
+  /**
+   * Convert an `Iterable` to a string like [IterableBase.toString].
+   *
+   * Allows using other delimiters than '(' and ')'.
+   *
+   * Handles circular references where converting one of the elements
+   * to a string ends up converting [iterable] to a string again.
+   */
+  static String iterableToShortString(Iterable iterable,

[floitsch, 2014/05/23 16:09:58]

Isn't this an exposed class?
Do you want to expose "iterableToShortString" ?

[Lasse Reichstein Nielsen, 2014/05/23 16:50:01]

Yes, I want to expose it.
I see no reason to have perfectly good functionality unexposed.
We think it's a reasonable default toString on Iterable, but we haven't made it
possible to use it on a Set or List, even if you wanted it (well, you could wrap
it in something that exposed the Set's iterator and used IterableMixin's
toString, but there is no easy way).

This way a user can decide to have a short, but still cycle-safe, toString of
their Set, like the default on IterableBase.

And no, the name isn't perfect. It's intended for programmers wanting to
implement some sub-type of Iterable, so it'll likely be hidden behind a more
meaningfull name (like toString). If you want to use it for debugging, that's
cool too.

+                                      [String leftDelimiter = '(',
+                                       String rightDelimiter = ')']) {
+    if (_toStringVisiting.contains(iterable)) {
+      if (leftDelimiter == "(" && rightDelimiter == ")") {
+        // Avoid creating a new string in the "common" case.
+        return "(...)";
+      }
+      return "$leftDelimiter...$rightDelimiter";
     }
-    String next = "${it.current}";
-    parts.add(next);
-    length += next.length + OVERHEAD;
-    count++;
+    List parts = [];
+    _toStringVisiting.add(iterable);
+    try {
+      _iterablePartsToStrings(iterable, parts);
+    } finally {
+      _toStringVisiting.remove(iterable);
+    }
+    return (new StringBuffer(leftDelimiter)
+                ..writeAll(parts, ", ")
+                ..write(rightDelimiter)).toString();
   }
 
-  String penultimateString;
-  String ultimateString;
+  /**
+   * Converts an `Iterable` to a string.
+   *
+   * Converts each elements to a string, and separates the results by ", ".
+   * Then wraps the result in [leftDelimiter] and [rightDelimiter].
+   *
+   * Unlike [iterableToShortString], this conversion doesn't omit any
+   * elements or puts any limit on the size of the result.
+   *
+   * Handles circular references where converting one of the elements
+   * to a string ends up converting [iterable] to a string again.
+   */
+  static String iterableToFullString(Iterable iterable,

[floitsch, 2014/05/23 16:09:58]

ditto.

[Lasse Reichstein Nielsen, 2014/05/23 16:50:01]

And yes, intended public.
I don't want anybody to ever cook up their own toString over a subclass of
Iterable and forget to handle cycles.

+                                     [String leftDelimiter = '(',
+                                      String rightDelimiter = ')']) {
+    if (_toStringVisiting.contains(iterable)) {
+      return "$leftDelimiter...$rightDelimiter";
+    }
+    StringBuffer buffer = new StringBuffer(leftDelimiter);
+    _toStringVisiting.add(iterable);
+    try {
+      buffer.writeAll(iterable, ", ");
+    } finally {
+      _toStringVisiting.remove(iterable);
+    }
+    buffer.write(rightDelimiter);
+    return buffer.toString();
+  }
 
-  // Find last two elements. One or more of them may already be in the
-  // parts array. Include their length in `length`.
-  var penultimate = null;
-  var ultimate = null;
-  if (!it.moveNext()) {
-    if (count <= HEAD_COUNT + TAIL_COUNT) return;
-    ultimateString = parts.removeLast();
-    penultimateString = parts.removeLast();
-  } else {
-    penultimate = it.current;
-    count++;
+  /** A set used to identify cyclic lists during toString() calls. */
+  static Set _toStringVisiting = new HashSet.identity();
+
+  /**
+   * Convert elments of [iterable] to strings and store them in [parts].
+   *
+   * This is the complicated part of [iterableToShortString].
+   * It is extracted as a separate function to avoid having too much code
+   * inside the try/finally.

[floitsch, 2014/05/23 16:09:58]

This should not be a dart-doc comment.

Feel free to leave it as a normal comment below the dart-doc, though.

[Lasse Reichstein Nielsen, 2014/05/23 16:50:01]

Why not. It's a private method, so it won't be exposed on api.dartlang.org, but
I sure hope the editor will show it as tooltip on the call, if you are editing
this file.

+   */
+  static void _iterablePartsToStrings(Iterable iterable, List parts) {
+    /// Try to stay below this many characters.
+    const int LENGTH_LIMIT = 80;
+    /// Always at least this many elements at the start.
+    const int HEAD_COUNT = 3;
+    /// Always at least this many elements at the end.
+    const int TAIL_COUNT = 2;
+    /// Stop iterating after this many elements. Iterables can be infinite.
+    const int MAX_COUNT = 100;
+    // Per entry length overhead. It's for ", " for all after the first entry,
+    // and for "(" and ")" for the initial entry. By pure luck, that's the same
+    // number.
+    const int OVERHEAD = 2;
+    const int ELLIPSIS_SIZE = 3;  // "...".length.
+
+    int length = 0;
+    int count = 0;
+    Iterator it = iterable.iterator;
+    // Initial run of elements, at least HEAD_COUNT, and then continue until
+    // passing at most LENGTH_LIMIT characters.
+    while (length < LENGTH_LIMIT || count < HEAD_COUNT) {
+      if (!it.moveNext()) return;
+      String next = "${it.current}";
+      parts.add(next);
+      length += next.length + OVERHEAD;
+      count++;
+    }
+
+    String penultimateString;
+    String ultimateString;
+
+    // Find last two elements. One or more of them may already be in the
+    // parts array. Include their length in `length`.
+    var penultimate = null;
+    var ultimate = null;
     if (!it.moveNext()) {
-      if (count <= HEAD_COUNT + 1) {
-        parts.add("$penultimate");
-        return;
-      }
-      ultimateString = "$penultimate";
+      if (count <= HEAD_COUNT + TAIL_COUNT) return;
+      ultimateString = parts.removeLast();
       penultimateString = parts.removeLast();
-      length += ultimateString.length + OVERHEAD;
     } else {
-      ultimate = it.current;
+      penultimate = it.current;
       count++;
-      // Then keep looping, keeping the last two elements in variables.
-      assert(count < MAX_COUNT);
-      while (it.moveNext()) {
-        penultimate = ultimate;
+      if (!it.moveNext()) {
+        if (count <= HEAD_COUNT + 1) {
+          parts.add("$penultimate");
+          return;
+        }
+        ultimateString = "$penultimate";
+        penultimateString = parts.removeLast();
+        length += ultimateString.length + OVERHEAD;
+      } else {
         ultimate = it.current;
         count++;
-        if (count > MAX_COUNT) {
-          // If we haven't found the end before MAX_COUNT, give up.
-          // This cannot happen in the code above because each entry
-          // increases length by at least two, so there is no way to
-          // visit more than ~40 elements before this loop.
+        // Then keep looping, keeping the last two elements in variables.
+        assert(count < MAX_COUNT);
+        while (it.moveNext()) {
+          penultimate = ultimate;
+          ultimate = it.current;
+          count++;
+          if (count > MAX_COUNT) {
+            // If we haven't found the end before MAX_COUNT, give up.
+            // This cannot happen in the code above because each entry
+            // increases length by at least two, so there is no way to
+            // visit more than ~40 elements before this loop.
 
-          // Remove any surplus elements until length, including ", ...)",
-          // is at most LENGTH_LIMIT.
-          while (length > LENGTH_LIMIT - ELLIPSIS_SIZE - OVERHEAD &&
-                 count > HEAD_COUNT) {
-            length -= parts.removeLast().length + OVERHEAD;
-            count--;
+            // Remove any surplus elements until length, including ", ...)",
+            // is at most LENGTH_LIMIT.
+            while (length > LENGTH_LIMIT - ELLIPSIS_SIZE - OVERHEAD &&
+                   count > HEAD_COUNT) {
+              length -= parts.removeLast().length + OVERHEAD;
+              count--;
+            }
+            parts.add("...");
+            return;
           }
-          parts.add("...");
-          return;
         }
+        penultimateString = "$penultimate";
+        ultimateString = "$ultimate";
+        length +=
+            ultimateString.length + penultimateString.length + 2 * OVERHEAD;
       }
-      penultimateString = "$penultimate";
-      ultimateString = "$ultimate";
-      length +=
-          ultimateString.length + penultimateString.length + 2 * OVERHEAD;
     }
-  }
 
-  // If there is a gap between the initial run and the last two,
-  // prepare to add an ellipsis.
-  String elision = null;
-  if (count > parts.length + TAIL_COUNT) {
-    elision = "...";
-    length += ELLIPSIS_SIZE + OVERHEAD;
-  }
-
-  // If the last two elements were very long, and we have more than
-  // HEAD_COUNT elements in the initial run, drop some to make room for
-  // the last two.
-  while (length > LENGTH_LIMIT && parts.length > HEAD_COUNT) {
-    String lastPart = parts.removeLast();
-    length -= lastPart.length + OVERHEAD;
-    if (elision == null) {
+    // If there is a gap between the initial run and the last two,
+    // prepare to add an ellipsis.
+    String elision = null;
+    if (count > parts.length + TAIL_COUNT) {
       elision = "...";
       length += ELLIPSIS_SIZE + OVERHEAD;
     }
+
+    // If the last two elements were very long, and we have more than
+    // HEAD_COUNT elements in the initial run, drop some to make room for
+    // the last two.
+    while (length > LENGTH_LIMIT && parts.length > HEAD_COUNT) {
+      length -= parts.removeLast().length + OVERHEAD;
+      if (elision == null) {
+        elision = "...";
+        length += ELLIPSIS_SIZE + OVERHEAD;
+      }
+    }
+    if (elision != null) {
+      parts.add(elision);
+    }
+    parts.add(penultimateString);
+    parts.add(ultimateString);
   }
-  if (elision != null) {
-    parts.add(elision);
-  }
-  parts.add(penultimateString);
-  parts.add(ultimateString);
 }