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..ca9d4ab72fed4b465dca35af84fadd0163754860 100644
--- a/lib/typed_buffers.dart
+++ b/lib/typed_buffers.dart
@@ -65,13 +65,29 @@ 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);
+  /// This adds values from [start] (inclusive) to [end] (exclusive) in
+  /// [values]. [start] defaults to `0` and [end] defaults to `values.length`.
+  void addAll(Iterable<E> values, [int start, int end]) {

[Lasse Reichstein Nielsen, 2015/10/15 10:58:25]

Use [int start = 0, int end] like other range parameters.
Basically we treat start as non-nullable and end as nullable.

Start out by checking that  start >= 0  and  end == null || end >= start.

[nweiz, 2015/10/15 20:14:10]

Done for consistency, although in general default values for non-bools cause
more pain than they're worth IME.
Done.

+    start ??= 0;
+    if (start == 0 && end == null) {
+      // `ListBase.insertAll` bottoms out on `setRange`, which has good handling
+      // of lists, typed buffers, and typed data lists.
+      insertAll(_length, values);

[Lasse Reichstein Nielsen, 2015/10/15 10:58:25]

You could also update insertAll to also take start/end, and just forward to that
in all cases.

[nweiz, 2015/10/15 20:14:10]

Done.

+      return;
+    }
+
+    var list = values is List ? values : values.toList(growable: false);

[Lasse Reichstein Nielsen, 2015/10/15 10:58:25]

I still don't want to convert the iterable to a list unnecessarily.
That's going to run through the iterable anyway, so we might as well do it
ourselves and avoid the intermediate allocation.

Maybe just:
  if (values is List) {
    // smart stuff
  } else {
    for (var value in values.getRange(start, end)) {
      _add(value);
    }
  }

You can get cleverer than that with the iterable (a tighter loop while there is
room in the buffer), but it likely isn't worth it.

[nweiz, 2015/10/15 20:14:10]

Done.

+    end = RangeError.checkValidRange(start, end, list.length);

[Lasse Reichstein Nielsen, 2015/10/15 10:58:25]

That requires the end to be inside the iterable. We don't generally require that
for iterables because the user is not expected to be able to test it
efficiently.
It should allow a too large start and end, treating it as if you had done
iterable.getRange(start, end).
The fact that we use a list internally is an optimization, but the semantics
should match iterables in general.

So:
  if (values is List) {
    if (start >= values.length) return;
    if (end == null || end > values.length) end = values.length;
    ... code below ...

[nweiz, 2015/10/15 20:14:10]

List.getRange doesn't allow too large a start or end. Iterable doesn't have a
getRange method, but Iterable.take method does allow too large an index. Also,
existing methods in the same vein like List.setRange *do* throw if the iterable
doesn't have enough elements—which is basically the same as end being too high.

[Lasse Reichstein Nielsen, 2015/10/20 08:09:49]

Generally we check the limits on lists and not on iterables (because the user
can check the list to avoid invalid lengths, but can't do it for iterables).
The List.setRange is inconsistent on that - probably because the range is really
on the list and the iterable is just expected to fill it. It's still not
optimal. I would actually consider fixing it (either stop early and return the
number of elements used, or fill the rest with a default value).

[nweiz, 2015/10/20 21:24:39]

Wouldn't this be a breaking change? The error is documented behavior.

I'd really prefer to match List.setRange's current behavior, even if it's
flawed. If it changes later, we can come back and change this as well. But if
you insist, I can change it now.

[floitsch, 2015/10/20 22:30:46]

Since one can leave "end" empty I prefer to check, too.
I almost prefer if skip and take were stricter, but then we would need to add a
boolean flag (or a different method).

+
+    var newLength = _length + end - start;
+    _growBuffer(newLength);

[Lasse Reichstein Nielsen, 2015/10/15 10:58:25]

I prefer _ensureCapacity. The function doesn't actually grow the buffer if the
needed capacity is already available, so the name is misleading.

[nweiz, 2015/10/15 20:14:10]

Done.

+    _setRange(_length, newLength, list, start);
+    _length = newLength;
   }
 
   void insert(int index, E element) {
@@ -92,6 +108,15 @@ abstract class _TypedDataBuffer<E> extends ListBase<E> {
     _buffer = newBuffer;
   }
 
+  /// Grows [_buffer] so that it's at least [requiredLength] long, preserving
+  /// the existing data.

[Lasse Reichstein Nielsen, 2015/10/15 10:58:25]

/// Ensures that [_buffer] is at least [requiredCapacity] long.
///
/// Grows the buffer if necessary.
void _ensureCapacity(int requiredCapacity) { ....

The parameter isn't the required length, it's the required capacity (actual
length may be greater).

[nweiz, 2015/10/15 20:14:10]

Done.
Okay, I'll also change [_createBiggerBuffer].

+  void _growBuffer(int requiredLength) {
+    if (requiredLength < _buffer.length) return;
+    var newBuffer = _createBiggerBuffer(null);
+    newBuffer.setRange(0, _length, _buffer);
+    _buffer = newBuffer;
+  }
+
   /**
    * Create a bigger buffer.
    *
@@ -116,6 +141,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 {