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

where can't accept an async predicate #56284

Closed
feinstein opened this issue Jul 19, 2024 · 6 comments
Closed

where can't accept an async predicate #56284

feinstein opened this issue Jul 19, 2024 · 6 comments
Labels
area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries. triage-automation See https://github.com/dart-lang/ecosystem/tree/main/pkgs/sdk_triage_bot. type-enhancement A request for a change that isn't a bug

Comments

@feinstein
Copy link

Consider this code:

set.where((item) async => await repository.getCount(item.id) < item.maxCount);

This will raise an error, because where needs a predicate that returns bool and not Future<bool>. Other methods from iterable.dart have the same issue.

I imagine one way of solving this is by creating a whereAsync that returns a Future<Iterable> instead.

@dart-github-bot
Copy link
Collaborator

Summary: The where method in Dart's iterable.dart library cannot accept an asynchronous predicate that returns a Future<bool>, causing an error. A potential solution is to introduce a whereAsync method that returns a Future<Iterable>.

@dart-github-bot dart-github-bot added area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries. triage-automation See https://github.com/dart-lang/ecosystem/tree/main/pkgs/sdk_triage_bot. type-enhancement A request for a change that isn't a bug labels Jul 19, 2024
@julemand101
Copy link
Contributor

There are already a solution for this if you import the package stream_transform from Dart team and you use Stream.fromIterable to convert your set into a Stream as a first step, which are basically what an async iterable are:

import 'package:stream_transform/stream_transform.dart';

void main() async {
  Set<String> set = {'One', 'Two', 'Three'};

  List<String> values = await Stream.fromIterable(set)
      .asyncWhere((value) =>
          Future.delayed(Duration(seconds: 1), () => value.length == 3))
      .toList();

  print(values); // [One, Two]
}

We should get a Stream regardless since Future<Iterable> is not a type that are going to be useful if we want to process each found element one at a time.

@feinstein
Copy link
Author

feinstein commented Jul 19, 2024

I considered this solution, but I think it's a waste of resources, as this would create a Stream and use the stream's "pipeline", which (I might be wrong), it's slower than a simple Iterable.

We should get a Stream regardless since Future is not a type that are going to be useful if we want to process each found element one at a time.

Not necessarily, in my case I want to filter a Set into another smaller Set, and pass this along to other parts of the app. My code has no idea what the rest of the app is going to do, but it knows it needs a Set. A Set has many unique properties, and those would be lost if I returned a Stream. So in my particular case, I would have to convert the Set into a Stream and back into a Set again, which as I said, I felt it was a waste of resources, compared to using the Iterable methods and then doing a Future.wait() then applying .toSet().

@julemand101
Copy link
Contributor

@feinstein Iterable are simple because you don't have any async nature around them. But that also means what you ask for makes it quite impossible to use Iterable since your asyncWhere on your Iterable cannot return another Iterable. You suggest it should return Future<Iterable> but that is not a great type to have since it would indicate that the Iterable have processed all its asyncWhere operations before we can continue using the Iterable.

But that removes a large part of using Iterable since we then no longer have a lazy evaluation flow of data. You are by this adding a lot of cost in memory since we need to do temporary buffering of events. So any talk about Stream might be heavier than Iterable does not really make sense since the stuff you want to add to Iterable removes the simplicity of Iterable.

Stream are not something I consider slow when we already are in a context of processing async events. Stream are also really what you should use if you ever think you want an async Iterable since that is what Streams really are.

I did also not suggest returning a Stream. I suggest using a Stream to do the asyncWhere and you can then call toSet() on the Stream to get your Future<Set>. Using Stream.fromIterable(set) does not convert the Set into some new collection structure. it really just create a Stream where the events comes from iterating over the provided set. And we then iterate over this Stream in a async way.

If you really don't want to deal with Streams, I think it is better for you to just create a custom function (maybe extension on Set for your code) where you then can have your asyncWhere that does what you want it to do.

But I don't think your proposed design (having asyncWhere on Iterable that returns Future<Iterable>) are a good fit for Dart.

@lrhn
Copy link
Member

lrhn commented Jul 19, 2024

No plans to add async support on base collections, or other non-asynchronous constructrs.
The result of an asynchronous .where on Set could not be Iterable.
It wouldn't even be Future<Iterable>, because that means every result is ready when it completes, so it might as well be Future<List>.
As @julemand101 says, what you seem to want here, based on the code without any context, is more like a Stream.

Asynchrony is something you opt in to, and then you choose where to put your asynchronous gaps, and how to combine asynchronous computations.

In this case, you can do any of the following:

Sequential checks using await on one future at a time.

Set result = {for (var item in set) if (await repository.getCount(item.id) < item.maxCount) item}

or

Set result = {};
for (var item in set) {
  if (await repository.getCount(item.id) < item.maxCount) result.add(item);
}

Parallel checks using .wait on a number of futures.

Future<Item?> ifValidCount(Item item) async => 
   (await repository.getCount(item.id)) < item.maxCount ? item : null;
Set result = (await set.map(ifValidCount).wait).nonNulls.toSet();

Use a stream to opt in to asynchrony.

Set result = await Stream.fromIterable(set).asyncWhere(
    (item) async => (await repository.getCount(item.id)) < item.maxCount).toSet();

There are many options, but they are not members of Set or Iterable, because those are simple data structures that have no idea asynchrony even exists. They're simply at a lower abstraction level. (Which is why it's safe to use iterables to implement asynchrony.)

@lrhn lrhn closed this as not planned Won't fix, can't repro, duplicate, stale Jul 19, 2024
@feinstein
Copy link
Author

Thanks for the thorough explanation, as always @lrhn.

I was indeed using map to convert to an iterable of Future, then a Future.wait on all futures, then a where and it seemed clumsy. I like many of the idiomatic ways you presented. There are so many cool new features in the language that I forget to use them sometimes, thanks a lot.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries. triage-automation See https://github.com/dart-lang/ecosystem/tree/main/pkgs/sdk_triage_bot. type-enhancement A request for a change that isn't a bug
Projects
None yet
Development

No branches or pull requests

4 participants