dart-lang / collection

The collection package for Dart contains a number of separate libraries with utility functions and classes that makes working with collections easier.
https://pub.dev/packages/collection
BSD 3-Clause "New" or "Revised" License
371 stars 85 forks source link

[Proposal] add mapSeparated method on `Iterable` #342

Open kishan-dhankecha opened 3 months ago

kishan-dhankecha commented 3 months ago

Current implementation using an extension method

extension MapSeparated<S> on Iterable<S> {
  Iterable<T> mapSeparated<T>(T separator, [T Function(S value)? generator]) sync* {
    final iterator = this.iterator;
    if (!iterator.moveNext()) return;
    yield generator?.call(iterator.current) ?? (iterator.current as T);
    while (iterator.moveNext()) {
      yield separator;
      yield generator?.call(iterator.current) ?? (iterator.current as T);
    }
  }
}

I chose to add this method as an extension of the Iterable type so I can use it for both List and Itrables. I mostly use this with List type and because of that I have to use either the spread operator to add output in another List or use it with toList(). We should discuss if this should stay as it is or we should add this only for List.

Use case:

Sometimes we want that list out our List or Iterable with something in between. For example, we have a List of Widget to use in the Column or Row with SizedBox(height: 10) but we are not using ListView.separated for some reason. There this can be useful!

Also, it can be useful when we are not sure whether each element of the Column or Row is displayed or not based on some condition and we want to add some space between two children.

eg.

...[
  if (review.content.title.isNotEmpty) ...[
    Text(
      review.content.title!.capitalize,
      style: const TextStyle(fontSize: 12, fontWeight: FontWeight.bold),
    ),
  ],
  if (review.content.review.isNotEmpty) ...[
    Text(
      review.content.review!.capitalize,
      maxLines: 5,
      overflow: TextOverflow.ellipsis,
      style: const TextStyle(fontSize: 12),
    ),
  ],
].mapSeparated<Widget>(const SizedBox(height: 4)),

Notice that I have used Iterable<S>, this can be useful when we have a List of type A and we want List of type B as output with separators. In the below example S is ({String text, Information info}) and We are using mapSeparated method to show all clickable TextSpan in RichText with , in between.

[
  (text: 'Terms of Use', info: Information.terms),
  (text: 'Privacy', info: Information.privacy),
  (text: 'Delivery', info: Information.delivery),
  (text: 'Auto-Ship', info: Information.autoship),
].mapSeparated(const TextSpan(text: ', '), (value) {
  return TextSpan(
    text: value.text,
    recognizer: value.info.tapRecognizer,
    style: const TextStyle(
      color: AppColor.primary,
      fontWeight: FontWeight.bold,
    ),
  );
}),

If anyone has any different views or take on this please share! I was using this with almost all of my projects hence I thought it should be added in here.

abitofevrything commented 3 months ago

After some discussion on the FlutterDev Discord server, a potential issue with mapSeparated is that T may be downwards inferred as a type not compatible with the iterator's type, in which case iterator.current as T will fail (generator must be provided in this case to avoid an error).

A solution would be to instead add a simpler separate(T separator) method, and simply call map prior to separate for users wanting to apply some transformation to the iterable's data.

Sample implementation:

extension <T> on Iterable<T> {
  Iterable<T> separate(T separator) sync* {
    final iterator = this.iterator;
    if (!iterator.moveNext()) return;
    yield iterator.current;
    while (iterator.moveNext()) {
      yield separator;
      yield iterator.current;
    }
  }
}
lrhn commented 3 months ago

A problem with this design is that if you have a List<TextWdget>, you can't separate those with SeparatorWidgets, because .separated must return something of the same type. So you'll do textWidgets.cast<Widget>(). separated(SeparatorWidget()).

That's not particularly convenient, a top level function taking both as arguments seems better.

Another issue is that you may not want to use the same instance as separator, and may want to create a new instance for each gap. Could have two versions, one taking a value, another a factory function. (Which could get the position as argument, in case it wants the individual separators to differ, say every fifth line is thicker.)

The most general version would probably be:

Iterable<E> separated<E>(Iterable<E> elements, Iterable<E> separators) sync* {
  var it = elements.iterator;
  if (!it.moveNext()) return; 
  yield it.current;
  if (!it.moveNext()) return; 
  for (var sep in separators) {
    yield sep;
    yield it.current;
    if (!it.moveNext()) return;
  }
  do {
    yield it.current;
  } while (it.moveNext());
}

which separates elements with separators as long as any are available.

gnprice commented 2 months ago

A problem with this design is that if you have a List<TextWdget>, you can't separate those with SeparatorWidgets, because .separated must return something of the same type. So you'll do textWidgets.cast<Widget>(). separated(SeparatorWidget()).

FWIW I think the idiomatic Flutter solution to that is that you should make the list's type be List<Widget> in the first place. If the list came from a list literal, then just write <Widget>[…]. If it came from a parameter, or a field on another widget, then that should probably take general Widget values; for example if a widget has a child field (or constructor parameter) then it's conventional for that to have type Widget even if only some types of widgets will actually make sense to use.

There can be exceptions to that pattern, but the fact they're uncommon may make it OK if they're slightly clunky to use with this method.

gnprice commented 2 months ago

which separates elements with separators as long as any are available.

I'm not sure how often one would have a use case for letting separators run out but then carrying on without separating. I suspect that if that happens, it'd most often be a bug, so that it'd be better for the method to throw in that case.

Conversely, if one does have a use case for separating the first $k$ items but not the ones after that, it seems like you might equally want to insert a separator only after every other item, or every 5 items, or some other pattern. I'm not sure that separating the first $k$ but not the rest is a sufficiently more canonical use case than those to be worth supporting.

(Maybe this is ultimately an argument for having it take a factory function, with the position as argument, instead of an iterable.)