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

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

Closed
aloisdg opened this issue Sep 20, 2021 · 13 comments
Closed
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation area-System.Linq untriaged New issue has not been triaged by the area owner

Comments

@aloisdg
Copy link

aloisdg commented Sep 20, 2021

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.

@aloisdg aloisdg added the api-suggestion Early API idea and discussion, it is NOT ready for implementation label Sep 20, 2021
@dotnet-issue-labeler dotnet-issue-labeler bot added the untriaged New issue has not been triaged by the area owner label Sep 20, 2021
@dotnet-issue-labeler
Copy link

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

aloisdg commented Sep 20, 2021

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

@aloisdg aloisdg changed the title [API Proposal]: Add a new signature to Sum() [API Proposal]: Add a new signature to Sum to be able to do: Sum((x, i) => ...) Sep 20, 2021
@ghost
Copy link

ghost commented Sep 20, 2021

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<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.

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.

Author: aloisdg
Assignees: -
Labels:

api-suggestion, area-System.Linq, untriaged

Milestone: -

@eiriktsarpalis
Copy link
Member

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

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

@stephentoub
Copy link
Member

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

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

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 (dotnet/csharplang#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
Copy link
Member

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

@tannergooding
Copy link
Member

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

CC. @jeffhandley

@jeffhandley
Copy link
Member

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

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

aloisdg commented Sep 28, 2021

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

@ghost ghost locked as resolved and limited conversation to collaborators Nov 3, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation area-System.Linq untriaged New issue has not been triaged by the area owner
Projects
None yet
Development

No branches or pull requests

7 participants