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]: Linq method for getting value and index #95563

Closed
terrajobst opened this issue Dec 4, 2023 · 11 comments · Fixed by #95947
Closed

[API Proposal]: Linq method for getting value and index #95563

terrajobst opened this issue Dec 4, 2023 · 11 comments · Fixed by #95947
Labels
api-approved API was approved in API review, it can be implemented area-System.Linq help wanted [up-for-grabs] Good issue for external contributors
Milestone

Comments

@terrajobst
Copy link
Member

terrajobst commented Dec 4, 2023

Background and motivation

There is an existing Linq method for returning an enumerable's value and index, but it was added pre-tuples which makes it clunky as it forces the caller to create a type representation for the combination of the value and index, for example using this:

IEnumerable<string> lines = GetLines();

foreach (var (line, lineIndex) in lines.Select((l, i) => (l, i)))
{
    // ...
}

API Proposal

namespace System.Linq;

public static partial class Enumerable
{
    public static IEnumerable<(T Value, int Index)> WithIndex<T>(this IEnumerable<T> source);
}

public static partial class Queryable
{
    public static IQueryable<(T Value, int Index)> WithIndex<T>(this IQueryable<T> source);
}

API Usage

Using WithIndex, one can express this idea much more succinctly:

IEnumerable<string> lines = GetLines();

foreach (var (line, lineIndex) in lines.WithIndex())
{
    // ...
}

Alternative Designs

No response

Risks

None.

@terrajobst terrajobst added the api-suggestion Early API idea and discussion, it is NOT ready for implementation label Dec 4, 2023
@ghost ghost added the untriaged New issue has not been triaged by the area owner label Dec 4, 2023
@ghost
Copy link

ghost commented Dec 4, 2023

Tagging subscribers to this area: @dotnet/area-system-linq
See info in area-owners.md if you want to be subscribed.

Issue Details

Background and motivation

There is an existing Linq method for returning an enumerable's value and index, but it was added pre-tuples which makes it clunky as it forces the caller to create a type representation for the combination of the value and index, for example using this:

IEnumerable<string> lines = GetLines();

foreach (var (line, lineIndex) in lines.Select((l, i) => (l, i)))
{
    // ...
}

API Proposal

namespace System.Linq;

public static partial class Enumerable
{
    public static IEnumerable<(T Value, int Index)> SelectIndex<T>(this IEnumerable<T> source);
}

public static partial class Queryable
{
    public static IQueryable<(T Value, int Index)> SelectIndex<T>(this IQueryable<T> source);
}

API Usage

Using SelectIndex, one can express this idea much more succinctly:

IEnumerable<string> lines = GetLines();

foreach (var (line, lineIndex) in lines.SelectIndex())
{
    // ...
}

Alternative Designs

No response

Risks

None.

Author: terrajobst
Assignees: -
Labels:

api-suggestion, area-System.Linq, untriaged

Milestone: -

@eiriktsarpalis
Copy link
Member

How about ZipIndex or ZipWithIndex? Select methods typically imply some form of transformation, however this one just zips the source enumerable with its indices (and has a similar output type to Zip).

@KeterSCP
Copy link

KeterSCP commented Dec 4, 2023

I also want to point out that name SelectIndex is very unintuitive. It implies that you select only index, which is not true. SelectWithIndex or ZipWithIndex are more verbose, but at least they do not introduce confusion

@marklio
Copy link

marklio commented Dec 4, 2023

How about just WithIndex? I agree that 'Select' implies transformation. 'Zip' implies starting with two things. This is just bringing an implicit attribute of the enumerable to the surface.

This is an extension that I have in almost all the code I write, so I'd love it if it were just ambiently available.

@terrajobst
Copy link
Member Author

@eiriktsarpalis that makes sense. Personally, I'd prefer @marklio's suggestion WithIndex over ZipIndex because it seems easier to understand.

@eiriktsarpalis
Copy link
Member

eiriktsarpalis commented Dec 5, 2023

Methods starting with With are typically associated with update expressions e.g. in fluent APIs or records. This is not in the same boat. Marking ready for review, I'm sure more than ample debate will be expended on the naming.

@eiriktsarpalis eiriktsarpalis removed the untriaged New issue has not been triaged by the area owner label Dec 5, 2023
@eiriktsarpalis eiriktsarpalis added this to the 9.0.0 milestone Dec 5, 2023
@eiriktsarpalis eiriktsarpalis added api-ready-for-review API is ready for review, it is NOT ready for implementation and removed api-suggestion Early API idea and discussion, it is NOT ready for implementation labels Dec 5, 2023
@aloraman
Copy link

aloraman commented Dec 5, 2023

  1. How about AsIndexed or just Indexed?
  2. What's the intended meaning of EventualNameOfThisMethod in IQueryable, e.g. should it be translated into ROW_NUMBER()?

@viceroypenguin
Copy link

The same method in MoreLinq and SuperLinq is just Index(). This is likely not a good choice for the name, but raising awareness of the community implementations of this.

@terrajobst
Copy link
Member Author

I'm sure more than ample debate will be expended on the naming.

@y0ung3r
Copy link

y0ung3r commented Dec 5, 2023

What about IndexableSelect or IndexedSelect? 😂

@terrajobst
Copy link
Member Author

terrajobst commented Dec 12, 2023

  • We decided against WithIndex because it sounds like a wither which this one isn't. F# calls it Indexed; following nomenclature in the rest of Linq we chose a verb version (akin to the recently added Order)
  • We decided to rename Value to Item because Value implies a singleton vs Item which is used as a collection indexer
  • We decided to return Index first, Value second because it feels more natural despite the fact that the existing Select() method provides value first and index second.
    IEnumerable<string> lines = GetLines();
    
    foreach (var (i, line) in lines.Index())
    {
        // ...
    }
  • We should match the existing Select's method behavior when enumerating a source that returns more than int.MaxValue items
namespace System.Linq;

public static partial class Enumerable
{
    public static IEnumerable<(int Index, T Item)> Index<T>(this IEnumerable<T> source);
}

public static partial class Queryable
{
    public static IQueryable<(int Index, T Item)> Index<T>(this IQueryable<T> source);
}

@terrajobst terrajobst added api-approved API was approved in API review, it can be implemented and removed api-ready-for-review API is ready for review, it is NOT ready for implementation labels Dec 12, 2023
@eiriktsarpalis eiriktsarpalis added the help wanted [up-for-grabs] Good issue for external contributors label Dec 12, 2023
@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Dec 13, 2023
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Dec 13, 2023
@github-actions github-actions bot locked and limited conversation to collaborators Jan 13, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api-approved API was approved in API review, it can be implemented area-System.Linq help wanted [up-for-grabs] Good issue for external contributors
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants