Refactor List.setRange function.

sdk/lib/core/list.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.core;
 
 /**
  * A [List] is an indexable collection with a length.
  *
  * A `List` implementation can choose not to support all methods
@@ skipping 223 matching lines @@
   /**
    * Returns an [Iterable] that iterators over the elements in the range
    * [start] to [end] (exclusive). The result of this function is backed by
    * `this`.
    *
    * It is an error if [end] is before [start].
    */
   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].
    * Throws an [UnsupportedError] if the list is
    * not extendable.
    * 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.
    */
   void removeRange(int start, int length);
@@ skipping 13 matching lines @@
   void insertRange(int start, int length, [E fill]);
 
   /**
    * Returns an unmodifiable [Map] view of `this`.
    *
    * It has the indices of this list as keys, and the corresponding elements
    * as values.
    */
   Map<int, E> asMap();
 }