AsyncGenerator icon indicating copy to clipboard operation
AsyncGenerator copied to clipboard

Support for async Select in LINQ

Open bahusoid opened this issue 7 years ago • 8 comments

Consider the following code in method that needs async counterpart:

CriteriaLoader[] loaders = ...
var loadedLists = loaders.Select(l => l.List(this)).ToList();

I expect this part to be converted in to something like:

CriteriaLoader[] loaders = ...
var loadedLists = loaders.Select(async l => await l.ListAsync(this, cancellationToken).ConfigureAwait(false)) ...?

But current behavior keeps this LINQ expression as is.

bahusoid avatar Apr 28 '18 04:04 bahusoid

Isn’t it #70?

hazzik avatar Apr 28 '18 05:04 hazzik

@bahusoid sorry - wrong number. Fixed now.

hazzik avatar Apr 28 '18 05:04 hazzik

Maybe - it's indeed about delegate parameters. But it's not obvious to me - suggested examples and comments in issue are about method generations with parameters transformation. To support async selects it seems we don't need any of it (at least if suggested example now make sense)

bahusoid avatar Apr 28 '18 05:04 bahusoid

Though it seems my suggestion still not valid (as it simply blocks thread execution instead awaiting). It seems the valid conversion can't be written in LINQ. It should be:

CriteriaLoader[] loaders = ...
var loadedLists = new List<IList>();
foreach (var l in loaders)
{
	loadedLists.Add(await l.ListAsync(this, cancellationToken));
}

Or am I wrong? Also I don't actually see how #70 can help here - could you please clarify?

bahusoid avatar Apr 28 '18 05:04 bahusoid

Also I don't actually see how #70 can help here - could you please clarify?

This feature is not related to #70 as the goal here is to convert the Select method that contains an async function into a foreach/for statement, where #70 is about converting method parameters.

For supporting Enumerable.Select we would have to convert it to a foreach/for statement as @bahusoid suggested. The problem with Select method is that it can be chained with other methods like OrderBy, Where and many others afterwards:

var result = loaders.Select(l => l.List(this)).Where(r => r.Count > 0).Select(r => r.First()).ToList();

The generator would need to known how to convert each method after the Select method. The converted code could look like:

var result = new List<object>();
foreach (var l in loaders) {
  var items = await l.ListAsync(this, cancellationToken).ConfigureAwait(false);
  if (items.Count > 0)
  {
    result.Add(items.First());
  }
}

Another example that we have to consider is when First, Single and other similar method are used:

var result = loaders.Select(l => l.List(this)).First();

Converting the above code to a foreach without breaking after the first iteration would cause a different behaviour as the above code will execute the List method only on the first element. In addition to that, we have to throw the same exception when loaders does not contain any element:

if (loaders.Length == 0)
{
  throw new InvalidOperationException("Sequence contains no elements");
}
IList result = null;
foreach (var l in loaders) {
  result = await l.ListAsync(this, cancellationToken).ConfigureAwait(false);
  break;
}

When an additional Skip method is used before the First method the we would have to use an additional variable for counting.

In short, supporting all scenarios would be very hard. Most likely I will initially add support for simpler cases and afterwards for complex ones.

maca88 avatar Apr 28 '18 09:04 maca88

To make a sustainable solution we might want to add SelectAsync method (but without IX/IObservable magic it would be ugly).

So this code:

var result = loaders.Select(l => l.List(this)).Where(r => r.Count > 0).Select(r => r.First()).ToList();

With naive SelectAsync:

public static async Task<IEnumerable<TResult>> SelectAsync<T, TResult>(
    this IEnumerable<T> enumerable,
    Func<T, Task<TResult>> selector)
{
    //TODO: What about infinity IEnumerable`s?
    var list = new List<TResult>();

    foreach (var item in enumerable)
        list.Add(await selector(item));

    return list;
}

Would become this:

var result = (await loaders.SelectAsync(l => l.ListAsync(this)))
.Where(r => r.Count > 0)
.Select(r => r.First())
.ToList();

With SelectAsync with some IX magic:

public static IAsyncEnumerable<TResult> SelectAsync<T, TResult>(
    this IEnumerable<T> enumerable,
    Func<T, Task<TResult>> selector)
{
    return AsyncEnumerable.CreateEnumerable(() =>
    {
        var enumerator = enumerable.GetEnumerator();
        var current = default(TResult);
        return AsyncEnumerable.CreateEnumerator(async c =>
            {
                var moveNext = enumerator.MoveNext();
                current = moveNext
                    ? await selector(enumerator.Current).ConfigureAwait(false)
                    : default(TResult);
                return moveNext;
            },
            () => current,
            () => enumerator.Dispose());
    });
}
var result = loaders.SelectAsync(l => l.ListAsync(this))
.Where(r => r.Count > 0)
.Select(r => r.First())
.ToList();

hazzik avatar Apr 29 '18 22:04 hazzik

After all, I think, that the best option would be to emit a warning/todo into the generated code saying that this code could potentially be converted to async, but without manual intervention this is not possible.

hazzik avatar Apr 29 '18 23:04 hazzik

Having a SelectAsync as @hazzik proposed would be the best option IMO, sadly IX doesn't provide one. It was proposed quite some time ago, but who knows when it will be implemented.

I think, that the best option would be to emit a warning/todo

The generator already does that, but there was a bug where it didn't worked for anonymous methods.

I am currently not sure if it is a good idea to rewrite an Enumerable.Select to a foreach/for statement as it could get quite complex and error prone, so I also thnik that having it logged is good enough for now.

maca88 avatar May 03 '18 18:05 maca88