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

Support for async Select in LINQ #94

Open
bahusoid opened this issue Apr 28, 2018 · 8 comments
Open

Support for async Select in LINQ #94

bahusoid opened this issue Apr 28, 2018 · 8 comments

Comments

@bahusoid
Copy link

bahusoid commented Apr 28, 2018

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.

@hazzik
Copy link
Contributor

hazzik commented Apr 28, 2018

Isn’t it #70?

@hazzik
Copy link
Contributor

hazzik commented Apr 28, 2018

@bahusoid sorry - wrong number. Fixed now.

@bahusoid
Copy link
Author

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

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?

@maca88
Copy link
Owner

maca88 commented Apr 28, 2018

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.

@hazzik
Copy link
Contributor

hazzik commented Apr 29, 2018

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

hazzik commented Apr 29, 2018

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.

@maca88
Copy link
Owner

maca88 commented May 3, 2018

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants