Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Proposal] add mapSeparated method on Iterable #674

Open
kishan-dhankecha opened this issue May 20, 2024 · 4 comments
Open

[Proposal] add mapSeparated method on Iterable #674

kishan-dhankecha opened this issue May 20, 2024 · 4 comments

Comments

@kishan-dhankecha
Copy link

kishan-dhankecha commented May 20, 2024

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
Copy link

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
Copy link
Member

lrhn commented May 31, 2024

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
Copy link

gnprice commented Jun 11, 2024

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
Copy link

gnprice commented Jun 11, 2024

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.)

@mosuem mosuem transferred this issue from dart-archive/collection Oct 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants