Change the toString method of IterableBase/IterableMixin to show some elements.

Change the toString method of IterableBase/IterableMixin to show some elements.

It always shows at least the first three and last two elements, unless the
iterable has more than 100 elements, in which case it's considered endless.
If there are room for more elements, within some limits, they are also included.

R=floitsch@google.com

Committed: https://code.google.com/p/dart/source/detail?r=28504

[Lasse Reichstein Nielsen, 2013-10-11 07:22:09]

PTAL

[floitsch, 2013-10-11 09:29:42]

LGTM.

https://codereview.chromium.org/26481002/diff/10001/sdk/lib/collection/hash_s...
File sdk/lib/collection/hash_set.dart (right):

https://codereview.chromium.org/26481002/diff/10001/sdk/lib/collection/hash_s...
sdk/lib/collection/hash_set.dart:60: List<E> result = growable ? (new
List<E>()..length = this.length)
I would have preferred if this was in a separate CL, but ok.

https://codereview.chromium.org/26481002/diff/10001/sdk/lib/collection/iterab...
File sdk/lib/collection/iterable.dart (right):

https://codereview.chromium.org/26481002/diff/10001/sdk/lib/collection/iterab...
sdk/lib/collection/iterable.dart:387: * Iterabele.
of `this`.

I don't like to repeat the name of the class. Subclasses get the inherited
documentation and there it frequently doesn't make sense.

https://codereview.chromium.org/26481002/diff/10001/sdk/lib/collection/iterab...
sdk/lib/collection/iterable.dart:398: * The conversion may omit calling
`toString` on some elements if they
Not sure we want to keep this sentence. If it's not specified it automatically
implies it.
But ok to leave.

https://codereview.chromium.org/26481002/diff/10001/sdk/lib/collection/iterab...
sdk/lib/collection/iterable.dart:422: while (length < LENGTH_LIMIT || count <
MIN_COUNT) {
nit. reorder:
while (count < MIN_COUNT || length < LENGTH_LIMIT)

It's the same, but to me the count is more important and should be first.

https://codereview.chromium.org/26481002/diff/10001/sdk/lib/collection/iterab...
sdk/lib/collection/iterable.dart:424: String next = "${it.current}";
it.current.toString() is more explicit.
But ok to keep.

https://codereview.chromium.org/26481002/diff/10001/sdk/lib/collection/iterab...
sdk/lib/collection/iterable.dart:437: if (count <= MIN_COUNT + 2) break
building;
Add comment?:
// No need for ellipsis.

https://codereview.chromium.org/26481002/diff/10001/sdk/lib/collection/iterab...
sdk/lib/collection/iterable.dart:454: // Then keep looping, keeping the last two
elements in variables.
"Then" doesn't fit.
Loop, keeping ...

https://codereview.chromium.org/26481002/diff/10001/tests/corelib/collection_...
File tests/corelib/collection_to_string_test.dart (right):

https://codereview.chromium.org/26481002/diff/10001/tests/corelib/collection_...
tests/corelib/collection_to_string_test.dart:239: print("CHECKING OMISSIONS");
debug print.

https://codereview.chromium.org/26481002/diff/10001/tests/corelib/collection_...
tests/corelib/collection_to_string_test.dart:243: print("$size>100");
debug print.

https://codereview.chromium.org/26481002/diff/10001/tests/corelib/collection_...
tests/corelib/collection_to_string_test.dart:256: print("$size<=100: $indices");
ditto

[Lasse Reichstein Nielsen, 2013-10-11 10:56:29]

Committed patchset #5 manually as r28504 (presubmit successful).

[Lasse Reichstein Nielsen, 2013-10-11 20:25:37]

https://codereview.chromium.org/26481002/diff/10001/sdk/lib/collection/hash_s...
File sdk/lib/collection/hash_set.dart (right):

https://codereview.chromium.org/26481002/diff/10001/sdk/lib/collection/hash_s...
sdk/lib/collection/hash_set.dart:60: List<E> result = growable ? (new
List<E>()..length = this.length)
Kept.

https://codereview.chromium.org/26481002/diff/10001/sdk/lib/collection/iterab...
File sdk/lib/collection/iterable.dart (right):

https://codereview.chromium.org/26481002/diff/10001/sdk/lib/collection/iterab...
sdk/lib/collection/iterable.dart:387: * Iterabele.
On 2013/10/11 09:29:43, floitsch wrote:
> of `this`.
> 
> I don't like to repeat the name of the class. Subclasses get the inherited
> documentation and there it frequently doesn't make sense.

Done.

https://codereview.chromium.org/26481002/diff/10001/sdk/lib/collection/iterab...
sdk/lib/collection/iterable.dart:398: * The conversion may omit calling
`toString` on some elements if they
I think it's worth emphasizing, to avoid someone thinking doing a toString will
always do a toString on each element, and they have some side effect that they
want to happen.

https://codereview.chromium.org/26481002/diff/10001/sdk/lib/collection/iterab...
sdk/lib/collection/iterable.dart:422: while (length < LENGTH_LIMIT || count <
MIN_COUNT) {
Done.
Although for efficiency, I'd expect the length check to hold more often than the
count check, so swapping them may be marginally slower.

https://codereview.chromium.org/26481002/diff/10001/sdk/lib/collection/iterab...
sdk/lib/collection/iterable.dart:424: String next = "${it.current}";
It may be slightly faster to call toString because it doesn't check that the
result is a String. And the VM may still be slow.
I'll switch to toString.

https://codereview.chromium.org/26481002/diff/10001/sdk/lib/collection/iterab...
sdk/lib/collection/iterable.dart:437: if (count <= MIN_COUNT + 2) break
building;
On 2013/10/11 09:29:43, floitsch wrote:
> Add comment?:
> // No need for ellipsis.

Done.

https://codereview.chromium.org/26481002/diff/10001/sdk/lib/collection/iterab...
sdk/lib/collection/iterable.dart:454: // Then keep looping, keeping the last two
elements in variables.
On 2013/10/11 09:29:43, floitsch wrote:
> "Then" doesn't fit.
> Loop, keeping ...

Done.

https://codereview.chromium.org/26481002/diff/10001/tests/corelib/collection_...
File tests/corelib/collection_to_string_test.dart (right):

https://codereview.chromium.org/26481002/diff/10001/tests/corelib/collection_...
tests/corelib/collection_to_string_test.dart:239: print("CHECKING OMISSIONS");
On 2013/10/11 09:29:43, floitsch wrote:
> debug print.

Done.

https://codereview.chromium.org/26481002/diff/10001/tests/corelib/collection_...
tests/corelib/collection_to_string_test.dart:243: print("$size>100");
On 2013/10/11 09:29:43, floitsch wrote:
> debug print.

Done.

https://codereview.chromium.org/26481002/diff/10001/tests/corelib/collection_...
tests/corelib/collection_to_string_test.dart:256: print("$size<=100: $indices");
On 2013/10/11 09:29:43, floitsch wrote:
> ditto

Done.