dotnet / runtime

.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps.
https://docs.microsoft.com/dotnet/core/
MIT License
15.26k stars 4.73k forks source link

[API Proposal]: Add a new signature to `Sum` to be able to do: `Sum((x, i) => ...)` #59336

Closed aloisdg closed 3 years ago

aloisdg commented 3 years ago

Background and motivation

Hello,

While writing a Luhn check algorithm, I realized that I often write a Select before doing a Sum to get the index. Like Select, Sum can already take a Func<TSource, int> selector as parameter but there is now way to get the index. We have to write:

source.Select((x, i) => (x, i)).Sum(t => t.x + t.i); 

Instead I would love to be able to just do:

source.Sum((x, i) => x + i); 

Of course, we will have to a new function per numeric type (e.g. decimal, double, float, etc.).

I am willing to take on implementing it.

API Proposal

public static int Sum<TSource>(this IEnumerable<TSource> source, Func<TSource, int, int> selector)
{
    if (source == null)
    {
        ThrowHelper.ThrowArgumentNullException(ExceptionArgument.source);
    }

    if (selector == null)
    {
        ThrowHelper.ThrowArgumentNullException(ExceptionArgument.selector);
     }

    int sum = 0;
    int index = 0;
    checked
    {
        foreach (TSource item in source)
        {
            sum += selector(item, index);
            index += 1;
        }
    }
    return sum;
}

I choose a naive implementation to be as close as possible to the existing code. The code would be written in Sum.cs

API Usage

var c = new []{1, 2, 3,};
var s = c.Sum((x, i) => i % 2 == 0 ? x * 2 : x);

// Getting the value out
Console.WriteLine(s); // 10

You can try this online.

Here is the code used for the demo:

public static class LinqExtensions
{
    public static int Sumi(this IEnumerable<int> source, Func<int, int, int> selector) => source.Select(selector).Sum();
}

Risks

I don't know any risks linked to this new signature. This contribution maintain API signature and behavioral compatibility. This contribution does not include any breaking changes.

dotnet-issue-labeler[bot] commented 3 years ago

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

aloisdg commented 3 years ago

I think a good label would be area-System.Linq.

ghost commented 3 years ago

Tagging subscribers to this area: @eiriktsarpalis See info in area-owners.md if you want to be subscribed.

Issue Details
### Background and motivation Hello, While writing a Luhn check algorithm, I realized that I often write a `Select` before doing a `Sum` to get the index. Like `Select`, `Sum` can already take a `Func selector` as parameter but there is now way to get the index. We have to write: ```cs source.Select((x, i) => (x, i)).Sum(t => t.x + t.i); ``` Instead I would love to be able to just do: ```cs source.Sum((x, i) => x + i); ``` Of course, we will have to a new function per numeric type (e.g. decimal, double, float, etc.). I am willing to take on implementing it. ### API Proposal ```C# public static int Sum(this IEnumerable source, Func selector) { if (source == null) { ThrowHelper.ThrowArgumentNullException(ExceptionArgument.source); } if (selector == null) { ThrowHelper.ThrowArgumentNullException(ExceptionArgument.selector); } int sum = 0; int index = 0; checked { foreach (TSource item in source) { sum += selector(item, index); index += 1; } } return sum; } ``` I choose a naive implementation to be as close as possible to the existing code. ### API Usage ```C# var c = new []{1, 2, 3,}; var s = c.Sum((x, i) => i % 2 == 0 ? x * 2 : x); // Getting the value out Console.WriteLine(s); // 10 ``` [You can try this online](https://dotnetfiddle.net/iAXnoi). Here is the code used for the demo: ```C# public static class LinqExtensions { public static int Sumi(this IEnumerable source, Func selector) => source.Select(selector).Sum(); } ``` ### Risks I don't know any risks linked to this new signature. This contribution maintain API signature and behavioral compatibility. This contribution does not include any breaking changes.
Author: aloisdg
Assignees: -
Labels: `api-suggestion`, `area-System.Linq`, `untriaged`
Milestone: -
eiriktsarpalis commented 3 years ago

Adding more LINQ method overloads comes at the expense of shared framework size, so before accepting a proposal we would need to see evidence that 1) the proposed addition solves a problem that many users are likely to encounter and 2) it is difficult to work around the problem in the absence of the proposed API.

Summation that takes the element index into account is a relatively niche application, and can be easily worked around using the approaches you proposed, or assuming you would like to avoid tuples:

source.Select((i,x) => i + x).Sum();

Alternatively, it should be fairly easy to roll an extension method of your own.

Furthermore, as you've already pointed out, for completeness this would require us adding overloads for every supported arithmetic type: int, int?, long, long?, float, float?, double, double, decimal and decimal?. So this would be adding substantial size to System.Linq without necessarily providing a lot of value to the majority of our users.

Therefore I don't believe this proposal would meet the bar for inclusion in System.Linq.

Clockwork-Muse commented 3 years ago

@eiriktsarpalis - what about adding an index to Aggregate()? (although in many cases passing along a temp object will be common)

stephentoub commented 3 years ago

Furthermore, as you've already pointed out, for completeness this would require us adding overloads for every supported arithmetic type

I agree with everything else you said, but for this specific claim I'd hope we could utilize a single generic overload constrained to one of the new arithmetic interfaces. cc: @tannergooding

eiriktsarpalis commented 3 years ago

what about adding an index to Aggregate()?

I think that question could be extended to any LINQ function that takes a delegate acting on individual elements: e.g. Any, GroupBy, First, etc. I suspect that our general approach here would be to recommend the .Select((i, x) => (i, x)) workaround, but we could certainly evaluate adding overloads in methods where accepting indices is demonstrably shown to be common practice.

tannergooding commented 3 years ago

but for this specific claim I'd hope we could utilize a single generic overload constrained to one of the new arithmetic interfaces.

Agreed. Noting that this would be dependent on checked operators (https://github.com/dotnet/csharplang/issues/4665) getting in so we could write a correct implementation.

We'd also need to determine if simply having IAdditionOperators is enough or if there are other qualifying factors we'd want to consider.

eiriktsarpalis commented 3 years ago

@tannergooding do we have an issue tracking API additions that take advantage of generic arithmetic?

tannergooding commented 3 years ago

Nope, I'd be fine with a label, project board, or similar provided there aren't any issues with doing that.

CC. @jeffhandley

jeffhandley commented 3 years ago

I recommend creating a public, org-level project board for this. That would allow us to collect proposals across all of our repositories, and look at them holistically across runtime, aspnetcore, and others.

eiriktsarpalis commented 3 years ago

In the meantime, I'm going to close this issue since the specific request does not meet the bar for inclusion in System.Linq. Thank you for your contribution @aloisdg.

aloisdg commented 3 years ago

@eiriktsarpalis you are welcome. Even if the request was rejected, I am glad to have submitted my first issue here :)