Refactor List.setRange function.

sdk/lib/core/list.dart

Index: sdk/lib/core/list.dart
diff --git a/sdk/lib/core/list.dart b/sdk/lib/core/list.dart
index b01209680f3171e4aada57ff4e78239768f02aa0..2c1d7422060f7f8c37ffb18683353d922956d049 100644
--- a/sdk/lib/core/list.dart
+++ b/sdk/lib/core/list.dart
@@ -241,15 +241,18 @@ abstract class List<E> implements Collection<E> {
   Iterable<E> getRange(int start, int end);
 
   /**
-   * Copies [length] elements of [from], starting
-   * at [startFrom], into the list, starting at [start].
-   * If [length] is 0, this method does not do anything.
-   * Throws an [ArgumentError] if [length] is negative.
-   * Throws an [RangeError] if [start] or
-   * [:start + length - 1:] are out of range for [:this:], or if
-   * [startFrom] or [:startFrom + length - 1:] are out of range for [from].
+   * Copies the elements of [from], starting
+   * at [startFrom], into the range [start] - [end] of `this`.
+   *
+   * If [start] equals [end], this method does not do anything.
+   *
+   * It is an error if [start]..[end] is not a valid range pointing into the
+   * `this`.
+   *
+   * It is an error if [startFrom] does not point into [from], or if [from]
+   * does not have enough elements.
    */
-  void setRange(int start, int length, List<E> from, [int startFrom]);
+  void setRange(int start, int end, List<E> from, [int startFrom]);
 

[Sean Eagan, 2013/04/11 14:22:07]

I think there is a lot to be learned from C#'s setRange here:

http://msdn.microsoft.com/en-us/library/system.collections.arraylist.setrange...

* The "end" parameter is redundant and thus error prone.  It should already be
deducible by "start" + "from.length".  When  you don't want all of "from", just
call "getRange", "skip", or "take" on it.  Same goes for "startFrom"

* "from" should be generalized to Iterable.


I would also consider:

* for non-fixed-length Lists, should it be allowed to set beyond the current
length?

* should "start" be optional and default to 0 to facilitate overwriting the
entire List?

* rename to "setAll" since the arguments are similar to "addAll", "removeAll",
and the hypothetical "insertAll" (http://dartbug.com/8924).


So something like:

void setAll(Iterable<E> all, [int start = 0]);

Usage:

list.setAll(otherList);
list.setAll(iterable.take(10), start);
list.setAll(otherList.getRange(10, 20), start);


Thanks!

[Lasse Reichstein Nielsen, 2013/04/11 14:33:51]

I generally agree with Sean.
If the is not the efficient low-level operation, then we might as well go full
Iterable on it and do:
  setSomething(Iterable source, [int at])
About growing the array if the source doesn't fit, I don't think it's a good
idea. There can be useful cases for writing up to the end, and throwing at that
point (we can't really do it earlier for a fixed-length list) would exactly give
this functionality to someone willing to catch-and-continue. Might as well make
it directly available.
So, perhaps better:
  int setSomething(Iterable source, [int at = 0])
that returns the number of elements written (or the index after the last one
written).
Then you can do:
  while (index < list.length) {
    index += list.setSomething(source, index);
  }
to fill the rest of a list with repeated content of the iterable.


(With that in mind, would replaceRange be better as:
   replaceRange(Iterable source, [int start = 0, int end])
so you can default to replacing everything?)

   /**
    * Removes [length] elements from the list, beginning at [start].