Add _TypedDataBuffer.addRange.

lib/typed_buffers.dart

Index: lib/typed_buffers.dart
diff --git a/lib/typed_buffers.dart b/lib/typed_buffers.dart
index 50ed2414c4cbd58d196a720e9706ecf31338a18e..4839884e210a980d1d3f1139aaeef3ae359e265b 100644
--- a/lib/typed_buffers.dart
+++ b/lib/typed_buffers.dart
@@ -65,13 +65,67 @@ abstract class _TypedDataBuffer<E> extends ListBase<E> {
     _buffer[_length++] = value;
   }
 
-  // We override the default implementation of `add` and `addAll` because
-  // they grow by setting the length in increments of one. We want to grow
-  // by doubling capacity in most cases.
+  // We override the default implementation of `add` because it grows the list
+  // by setting the length in increments of one. We want to grow by doubling
+  // capacity in most cases.
   void add(E value) { _add(value); }
 
-  void addAll(Iterable<E> values) {
-    for (E value in values) _add(value);
+  /// Appends all objects of [values] to the end of this buffer.
+  ///
+  /// This adds values from [start] (inclusive) to [end] (exclusive) in
+  /// [values]. If [end] is omitted, it defaults to adding all elements of
+  /// [values] after [start].
+  ///
+  /// Throws a [StateError] if [values] isn't long enough to extract a slice
+  /// between [start] and [end].
+  void addAll(Iterable<E> values, [int start = 0, int end]) {
+    insertAll(_length, values, start, end);

[Lasse Reichstein Nielsen, 2015/10/23 08:00:02]

I generally prefer to not define one public API method in terms of another
because it's fragile if someone tries to overload both (e.g., add logging to the
call, then you get logging for both).
In cases like this, I'd use a private _insertAll called from here and from
insertAll.

[nweiz, 2015/10/26 23:53:57]

Done.

+  }
+
+  /// Inserts all objects of [values] at position [index] in this list.
+  ///
+  /// This adds values from [start] (inclusive) to [end] (exclusive) in
+  /// [values]. If [end] is omitted, it defaults to adding all elements of
+  /// [values] after [start].
+  ///
+  /// Throws a [RangeError] if [index] is less than 0 or greater than [length].
+  /// Throws a [StateError] if [values] isn't long enough to extract a slice

[Lasse Reichstein Nielsen, 2015/10/23 08:00:02]

Don't specify the exact errors, just:
 
 The [start] value must be non-negative.
 The [values] iterable must have at least [start] elements, and if
 [end] is specified, it must be greater than or equal to [start] and
 [values] must have at least [end] elements.

[nweiz, 2015/10/26 23:53:57]

Done, but why? Isn't it good for the user to know what errors to expect?

[floitsch, 2015/10/27 10:26:17]

It really matters for exceptions, but not for errors. The error-class should
only matters for representation. That is, how the toString works, and what it
captures to give better debugging information.
The program itself should never care.
By not specifying which error, it allows us to switch to a better fitting error
in the future (if we introduce one).

[Lasse Reichstein Nielsen, 2015/10/27 10:33:12]

What Florian said!

+  /// between [start] and [end].
+  void insertAll(int index, Iterable<E> values, [int start = 0, int end]) {

[Lasse Reichstein Nielsen, 2015/10/23 08:00:02]

Maybe split this earlier if [values] is a list:

  if (values is List) {
    end = RangeError.checkValidRange(start, end, values.length);
    ... do the list thing ...
    return;
  } else {
    _insertAllIterable(index, values, start, end);
  }

  void _insertAllIterable(index, values, start, end) {
    ... do the iterable checks and insertion.
  

With a risk of being inconsistent, doing a toList on the iterable does make
sense for insertAll when it's not at the end. At the end, it is still possible
to add iteratively without knowing the count, but inserting in the middle has
potential non-linear behavior in that case.

So, iterating on the design, I still want start/end on both addAll and
insertAll, but maybe it's insertAll that should delegate to addAll when the
insertion index is at the end.

  void insertAll(...) {
    if (index == length) {
      _addAll(valus, start, end);
      return;
    }
    int originalStart = start;
    int originalEnd = end;
    if (values is! List) { 
      if (end != null) {
        values = values.take(end);
        end -= start;
      }
      values = values.skip(start).toList();
      start = 0;
    } 
    end = RangeError.checkValidRange(origialStart, originalStart, 
              list.length + originalStart) - originalStart;
    ... efficient list-based insert and range checks ...
  }
  void addAll(Iterable values, [start = 0, end]) {
    _addAll(values, start, end);
  }
  void _addAll(...) {
     if (values is List) {
        efficient list based add.
     } else {
       RangeError.checkNotNegative(start, "start");
       if (end != null) {
         RangeError.checkValidRange(end, start, null, "end");
         values = values.take(end);
       }
       int i = 0;  
       for (var value in values) {
         if (i >= start) _add(value);
         i++;
       }
       if (i < start) { 
         throw new RangeError.range(start, 0, i, "start");
       }
       if (end != null && i < end) {
         throw new RangeError.range(end, start, i, "end");
       }
     }
  }

[nweiz, 2015/10/26 23:53:57]

Done.

+    RangeError.checkValidIndex(index, this, "index", _length + 1);
+    RangeError.checkNotNegative(start, "start");
+    if (end != null && start > end) {
+      throw new RangeError.range(end, start, null, "end");
+    }
+
+    // If [values] is an iterable, we can't efficiently get its length. If we
+    // also don't have [end], we don't know how large to expand [_buffer] so we
+    // fall back to the default implementation.
+    if (end == null && values is! List) {
+      var i = 0;
+      for (var value in values) {
+        if (i >= start) insert(index + i - start, value);
+        i++;
+      }
+      if (i < start) throw new StateError("Too few elements");

[Lasse Reichstein Nielsen, 2015/10/23 08:00:02]

It's the [values] argument that have too few arguments, not "this", so I'd make
it an argument error not a state error.
For consistency, I'd do:
  new RangeError.range(start, 0, i, "start");

[nweiz, 2015/10/26 23:53:57]

This is inconsistent with the corresponding error raised by setRange:
https://github.com/dart-lang/sdk/blob/master/sdk/lib/collection/list.dart#L380

[Lasse Reichstein Nielsen, 2015/10/27 10:33:12]

So it is.
I'm getting less and less enchanted by the setRange implementation.

+      return;
+    }
+
+    if (values is List) {
+      end ??= values.length;
+      if (start > values.length || end > values.length) {
+        throw new StateError("Too few elements");

[Lasse Reichstein Nielsen, 2015/10/23 08:00:02]

RangeError.range(end, start, values.length, "end");

+      }
+    }
+
+    var valuesLength = end - start;
+    var newLength = _length + valuesLength;
+    _ensureCapacity(newLength);
+
+    _buffer.setRange(
+        index + valuesLength, _length + valuesLength, _buffer, index);
+    _buffer.setRange(index, index + valuesLength, values, start);
+    _length = newLength;
   }
 
   void insert(int index, E element) {
@@ -92,18 +146,28 @@ abstract class _TypedDataBuffer<E> extends ListBase<E> {
     _buffer = newBuffer;
   }
 
+  /// Ensures that [_buffer] is at least [requiredCapacity] long,
+  ///
+  /// Grows the buffer if necessary, preserving existing data.
+  void _ensureCapacity(int requiredCapacity) {
+    if (requiredCapacity <= _buffer.length) return;
+    var newBuffer = _createBiggerBuffer(requiredCapacity);
+    newBuffer.setRange(0, _length, _buffer);
+    _buffer = newBuffer;
+  }
+
   /**
    * Create a bigger buffer.
    *
    * This method determines how much bigger a bigger buffer should
-   * be. If [requiredLength] is not null, it will be at least that
+   * be. If [requiredCapacity] is not null, it will be at least that
    * size. It will always have at least have double the capacity of
    * the current buffer.
    */
-  List<E> _createBiggerBuffer(int requiredLength) {
+  List<E> _createBiggerBuffer(int requiredCapacity) {
     int newLength = _buffer.length * 2;
-    if (requiredLength != null && newLength < requiredLength) {
-      newLength = requiredLength;
+    if (requiredCapacity != null && newLength < requiredCapacity) {
+      newLength = requiredCapacity;
     } else if (newLength < INITIAL_LENGTH) {
       newLength = INITIAL_LENGTH;
     }
@@ -116,6 +180,11 @@ abstract class _TypedDataBuffer<E> extends ListBase<E> {
 
   void setRange(int start, int end, Iterable<E> source, [int skipCount = 0]) {
     if (end > _length) throw new RangeError.range(end, 0, _length);
+    _setRange(start, end, source, skipCount);
+  }
+
+  /// Like [setRange], but with no bounds checking.
+  void _setRange(int start, int end, Iterable<E> source, int skipCount) {
     if (source is _TypedDataBuffer<E>) {
       _buffer.setRange(start, end, source._buffer, skipCount);
     } else {