Ambiguous ReadAllConcurrentlyAsync signatures, when using named arguments

[dittodhole]

Hi there,

please consider following snippet:

channel.ReadAllConcurrentlyAsync(maxConcurency: ?,
                                 receiver: ?,
                                 cancellationToken: ?)

This call will fail with CS0121.

Referenced methods:

• https://github.com/Open-NET-Libraries/Open.ChannelExtensions/blob/314f382784e0e54896815f30421e91a0aa793943/Open.ChannelExtensions/Extensions.ReadConcurrently.cs#L22
• https://github.com/Open-NET-Libraries/Open.ChannelExtensions/blob/314f382784e0e54896815f30421e91a0aa793943/Open.ChannelExtensions/Extensions.ReadConcurrently.cs#L84

Additionally, TaskReadAllConcurrentlyAsync is also affected.

[electricessence]

I see. For the case where optional params are normalized, this begins to fail. I'm torn here. As you will always need to specify a max concurrency and a delegate. They are not optional. Only the cancellation token is optional.

The reason for theirs structure as they are is to simplify the code at use. Example:

reader.ReadAllConcurrentlyAsync(3, token, async e => {
});

instead of...

reader.ReadAllConcurrentlyAsync(3, async e => {
}, token);

Where the former has the execution as the last param/statement.

I thought about this quite a bit when making library, and started with the foundation of (maxConcurrency, receiver, cancellationToken = default) but found myself using the other signature more often for readability.

I'm open to suggestions. I have a few not so great ideas in my head, but altering the current API would likely break things. Thoughts?

[dittodhole]

Hm, I see ... thanks for the historical background!

Having an optional CancellationToken parameter as the last parameter is the signature I am used to from other enterprise projects (see Quartz.NET extending to async, see Polly with their ExecuteAsync signatures, ...), and I find that doing myself quite often (as it follows the "default" signature style). Additionally, the transform delegate could also share the CancellationToken.

Anyway, "2 methods doing the same thing" justify a breaking change imho (and might not be that big deal following SemVer)

[electricessence]

Yes. Normally I would follow the convention. But this only breaks when you are doing something very specific. Anyways,.. Hope this lib is helpful to you.

[dittodhole]

Just shared my thoughts (as requested), and acknowledge your point (no offense intended). The library is of tremendous help, just wanted to point out some (minor!, and from my modest experience) unusual design decisions.

[electricessence]

Appreciate it. I did struggle to make the puzzle work, and maybe in hindsight I might have come up with a way to retain the standard. But I can tell you, all the FxCop suggestions were there, I just decided on the most common use case for a fluent API.