zip can only zip iterables of the same type

[mreichelt]

The zip method is especially useful when we can zip items of different types - but zip in dartx is implemented to only accept a right-hand iterable of the same type. So things like this work:

final numbers = [1, 2, 3];
final multipliers = [3, 4, 5];

final multiplied = numbers.zip(multipliers, (number, multiplier) => number * multiplier).toList();
print(multiplied); // prints [3, 8, 15]

But this doesn't:

final countOfFruits = [1, 2, 3];
final fruits = ['bananas', 'apples', 'oranges'];
final combined = countOfFruits.zip(fruits, (count, fruit) => '$count $fruit').toList();

We can try to change the implementation of the original zip, but then we must make sure we don't break any calling code. So let's test this thoroughly. 🤓

[mreichelt]

Rough new implementation that works for me:

extension IterableZip<E> on Iterable<E> {
  Iterable<V> zipWith<R, V>(Iterable<R> other, V Function(E a, R b) transform) sync* {
    final it1 = iterator;
    final it2 = other.iterator;
    while (it1.moveNext() && it2.moveNext()) {
      yield transform(it1.current, it2.current);
    }
  }
}

[mreichelt]

Yep - this works, but unfortunately it also breaks consumer code:

The second parameter will now become dynamic - which is sad, because every info for Dart to find out the generic R is available at build time. It just won't pass that as generic for parameter b in transform.
And Dart doesn't have operator overloading, so that doesn't work as well.

@leisim @passsy it would be great if you can add some guidance on how you would like this to be handled. I basically see two options:

1. Keep the existing implementation, and add the new one under a different name (i.e. zipWith). That works, but then we'll have two methods where one is more broad than the other. And we'll loose the nice naming, which I would like to stay similar with Kotlin. We could deprecate zip, but still we would not be able to keep the name.
2. Leverage the fact that dartx is still a 0.x version, and as such, can add breaking changes. Maybe we should ask the community first before we do this. I added a PR for this, just to show how this would look like and to start the discussion.

[simc]

@mreichelt Thanks for the detailed proposal.

Is it only me or is this a bug in Dart? In my opinion Dart should be able to infer the types correctly without manual annotations.

I don't think we should add a zipWith() method that basically does the same as the zip() method. It is not great that type annotations will be necessary in the future but I think we should replace the existing zip() method with the proposed one.

[mreichelt]

Ok, thanks! Then, at least I'll add some docs to the zip method later so people know how to make their code working again :)

[mreichelt]

@leisim I added an example in the .zip() documentation as stated. It's ready to be merged now IMO :-)
#94

[passsy]

That's a problem I've also ran into when building kt.dart.

Definitely a dart issue

I think right now would be a good time to open a dart-lang/sdk issue about this. I searched for quite a while but haven't found anything related.

details

First guesses:
It seems like Kotlin evaluates generic type parameters (T) in the arguments starting with the first argument. If types are provided, it will assume those types for all following arguments, also when an argument is a lambda of type (T) -> Any
In Dart the generic types in the arguments don't seem to have precedence. The compiler doesn't forward previously defined generic types (T) to lambdas which are defined as dynamic Function(dynamic).

TODO: come up with an easier example than zip() for the bug report

Split zip in two methods

Since we're doing a breaking API change, I was thinking about aligning the zip functionality from kt_dart with dartx.

In kt_dart I've split zip into two methods

• zip<R>(KtIterable<R> other): KtList<KtPair<T, R>> src
  Returns a pair with T and R
• zipTransform<R, V>(KtIterable<R> other, V Function(T a, R b) transform): KtList<V> src
  Same as the proposed zip here.

I decided against it because Dart doesn't offer tuples in the Dart SDK. Therefore we are left with a single zip method and don't have to think about the naming conflict.

Going forward

• Let's do the breaking change
• Create a Dart SDK bug report
• Document the new required type information adequately
• Wait for Dart SDK fix to make our documentation obsolete