Fix IAsyncEnumerable SkipLast for count = 0 by thibault-reigner · Pull Request #1782 · dotnet/reactive
I see...so there is a case in which we have to return
sourceunchanged but have to 'hide' the object identity. I wasn't aware of that.I am thinking about not touching the implementation of SkipLast, but instead having a generic way to wrap
sourcebut keep its behaviour.AsAsyncEnumerablecomes to mind, but that'll only return the object itself.Is there more cases like this one, where
Coreis called only to hidesource? If so, could you provide a more generic solution, like e.g. aWrapmethod? I think that would be the cleanest solution because we wouldn't touch any implementations and correctness of the changes would be immediately obvious.In any case, good find!
I see...so there is a case in which we have to return
sourceunchanged but have to 'hide' the object identity. I wasn't aware of that.I am thinking about not touching the implementation of SkipLast, but instead having a generic way to wrap
sourcebut keep its behaviour.AsAsyncEnumerablecomes to mind, but that'll only return the object itself.Is there more cases like this one, where
Coreis called only to hidesource? If so, could you provide a more generic solution, like e.g. aWrapmethod? I think that would be the cleanest solution because we wouldn't touch any implementations and correctness of the changes would be immediately obvious.In any case, good find!
This seems to be a bit out of scope of this PR and quite some work, as it requires to search for such pattern in all extensions methods of IAsyncEnumerable.
Also, I noticed that there is an extensive use of GetAsyncEnumerator() + MoveNextAsync() instead of await foreach in this library, most likely due to the fact it predates the introduction of IAsyncEnumerable and C# 8 await foreach.
It led to extra complexity, and here bug, with no easy way to compare methods with their synchronous variations for IEnumerable (which here was bug-free), so maybe a transition would highlight some extra bugs but also potential factorization.
Do you think such rework would be worth it to spot places where some simplfying logic such as the Wrap you suggest are relevant ?