Runtime: Allow for specify return value on System.Linq.Enumerable.*OrDefault methods

Created on 31 Jan 2017  路  33Comments  路  Source: dotnet/runtime

I would like to propose adding overloads to the System.Linq.Enumerable.OrDefault and System.Linq.Queryable.OrDefault methods for specifying the value to be returned in the default case. This is the value that would be returned instead of default(TSource) when there are no items in the enumeration

There are times when the default value of a given type may be a valid value from the enumeration. If the default value for the enumeration type is valid value, there is not a nice way to determine if the returned values was because the enumeration was empty or if that value was in the enumeration.

Proposed API change

```C#
namespace System.Linq
{
public static class Enumerable
{
public static TSource SingleOrDefault(this IEnumerable source, TSource defaultValue);
public static TSource SingleOrDefault(this IEnumerable source, Func predicate, TSource defaultValue);

    public static TSource FirstOrDefault<TSource>(this IEnumerable<TSource> source, TSource defaultValue);
    public static TSource FirstOrDefault<TSource>(this IEnumerable<TSource> source, Func<TSource, bool> predicate, TSource defaultValue);

    public static TSource LastOrDefault<TSource>(this IEnumerable<TSource> source, TSource defaultValue);
    public static TSource LastOrDefault<TSource>(this IEnumerable<TSource> source, Func<TSource, bool> predicate, TSource defaultValue);
}

public static class Queryable
{
    public static TSource SingleOrDefault<TSource>(this IQueryable<TSource> source, TSource defaultValue);
    public static TSource SingleOrDefault<TSource>(this IQueryable<TSource> source, Func<TSource, bool> predicate, TSource defaultValue);

    public static TSource FirstOrDefault<TSource>(this IQueryable<TSource> source, TSource defaultValue);
    public static TSource FirstOrDefault<TSource>(this IQueryable<TSource> source, Func<TSource, bool> predicate, TSource defaultValue);

    public static TSource LastOrDefault<TSource>(this IQueryable<TSource> source, TSource defaultValue);
    public static TSource LastOrDefault<TSource>(this IQueryable<TSource> source, Func<TSource, bool> predicate, TSource defaultValue);
}

}
```

Updates

  • Switch from using default parameters to method overloads
  • Added IQueryable methods as well
api-approved area-System.Linq up-for-grabs

Most helpful comment

@Joe4evr Good catch. Since they were approved as "add TSource defaultValue to existing overloads" I've gone ahead and fixed the approved shape to be sensible.

All 33 comments

This would be an alternative to having to switch from SingleOrDefault(predicate) to DefaultIfEmpty(x).Single(predicate).

You are adding new methods (we can't change existing ones).
I think the = default(TSource) will cause compiler ambiguity with the existing methods - we need to drop the default value IMO (please check first if I am not mistaken).

It wouldn't cause ambiguity at the compiler level, but the default would never be used because the existing methods would always beat it in the overload resolution, so it's pointless. It could certainly cause ambiguity at the human understanding level.

If we had this (I've no strong opinion either way on that) we should also have Queryable equivalents.

I believe the way these conversations usually go is that you can't use optional parameters without breaking binary back compat. http://haacked.com/archive/2010/08/10/versioning-issues-with-optional-arguments.aspx

I think it would be safe in that regard, given the types involved, (though it could break someone reflecting and using the count of parameters to find which overload they wanted), but I wouldn't bet on that, and it could still mess with some future change, and in any case they're still pointless defaults as they'll never be used.

@karelz and @JonHanna I updated the issue description based on your comments (I think I did it correctly; first time filing an issue in this repo).

@Keboo all good so far, we will let you know if you're not following "the rules" ;-)

FYI: Next steps in API review process are:

  1. Feedback from community on API addition
  2. Area owners support the API addition as good & valuable and tag it as 'api-ready-for-review'
  3. API review group reviews the API and tags it as 'api-approved' (or sends it back to 'api-needs-work' with comments

Don't expect super-fast turnaround. New APIs need bake time and some thinking through. The process should not be rushed even for obvious things.

If you are eager to contribute to CoreFX, I would recommend to grab some 'up for grabs' issues (which are not API proposals) in the meantime - the most valuable/impactful things are:

  1. 2.0.0 (next release) up for grabs issues
  2. wishlist issues
  3. simple issues for newbie contributors (not applied too much yet - we just started experimenting with it recently)

@jnm2 AFAIK the proposal only adds APIs, it of course can't remove any ;-)

@karelz Ah, misunderstood the purpose of the original proposal. Latest revision makes sense.

Would it be better to have a selector run for the default value instead of a concrete value?

public static TSource FirstOrDefault<TSource>(this IEnumerable<TSource> source, Func<TSource> selectorIfEmpty);

I wonder if we could also pass in more information to the SingleOrDefault selector, like whether the enumerable was empty or had more than 1 element.

@jamesqo I am not sure the selector adds much value here. Can you elaborate on when you would find it useful?
As for the SingleOrDefault case, it sounds like you are expecting this selector to get invoked in both when the enumeration is empty and when it contains multiple items. Wouldn't this be a change in the behavior of SingleOrDefault?

This makes sense to me... good suggestion @Keboo.

@Keboo Sorry for the late reply.

I am not sure the selector adds much value here. Can you elaborate on when you would find it useful?

I was thinking it could be used to lazily load a value if there was no item matching the predicate. Looking back though, I agree it may not make much sense, mainly because Linq is used with pure functions so a argument-less function would go against the grain.

@karelz Everyone seems to think this is a good idea, so maybe it can be marked ready-for-review. (I have another idea for these methods which I plan to open a proposal for later, but I won't block this on it.)

@jamesqo area experts are supposed to make the first-level API approval, by marking the API as such: @VSadov @OmarTawfik can you please check if the API proposal is in line with Linq direction?

LGTM. @VSadov?
I think it is useful addition to the API.

This is a valuable addition, LGTM as proposed. @adamsitnik any objections to marking this for api review?

@eiriktsarpalis I agree that the problem exists and it should be addressed. Example:

IEnumerable<int> integers = SomehowGetACollectionOfIntegers();
int item = integers.FirstOrDefault(); 

if (item == 0) // was the first value a `0` or there was no values at all?

However, I am not sure if passing a default value to the method is the best solution as it forces the user to perform one extra comparison. It sounds easy, but:

  • if the given type does not implement IEqutable<T> interface, then the extra check might introduce another set of problems that are related to the usage of object.Equals(object):

    • reference comparison for reference types and value comparison for value types (it's often surprising for new C# devs)

    • boxing of value types and virtual method calls (performance)

  • in a generic context, the user can not use the == operator (usability)
  • if we add T : IEqutable<T> generic constraint then it limits the usage for cases where given type implementation can not be changed (usability)

nit: Perhaps Given would be a better name than Default?

- TSource SingleOrDefault<TSource>(this IEnumerable<TSource> source, TSource defaultValue);
+ TSource SingleOrGiven<TSource>(this IEnumerable<TSource> source, TSource given);

Alternatives are:

  • Try pattern which does not fit LINQ very well (not intuitive, harder to chain multiple method calls)
bool TryFirst<TSource>(this IEnumerable<TSource> source,  out TSource found);

and could also be unclear what false means in case of methods like TrySingle where false could mean that there was more than one value or no values at all.

  • returning tuple (bool found, TSource result). Personally, this is my favorite. But it would most probably require a new method name and I don't have a good proposal
IEnumerable<int> integers = SomehowGetACollectionOfIntegers();
var item = integers.FirstOrDefault(); // compiler  error: did you mean (int) or (bool, int)

@eiriktsarpalis what do you think? I would like to be well prepared for the API review.

I am not sure if passing a default value to the method is the best solution as it forces the user to perform one extra comparison.

I am not sure why you would need any form of comparison here. For example, a naive implementation of FirstOfDefault would be the following:

public static TSource FirstOrDefault<TSource>(this IEnumerable<TSource> source, TSource defaultValue)
{
    foreach (TSource element in source)
    {
        return element;
    }

    return defaultValue;
}

I am not sure why you would need any form of comparison here

You are right. I've missunderstood the idea. It's "give me X or use this" and most probably no ifs after that.

@eiriktsarpalis what about the name? do you like the idea of changing Default to Given?

do you like the idea of changing Default to Given?

Probably, Default means a very specific thing which does not hold in the proposed overloads. Probably something to bring up during API review.

@eiriktsarpalis good, then I've changed the label from api-needs-work to api-ready-for-review

  • Try pattern which does not fit LINQ very well (not intuitive, harder to chain multiple method calls)

All of the proposed methods end the chain.

nit: Perhaps Given would be a better name than Default?

To add my $0.02, I had suggested Else in #33443 for similar reasons you state. However, the OrElse paradigm has a history in other languages and is what I would expect for these endpoints. FirstOrGiven seems rather alien in that sense.

Ultimately I feel either would be fine, but I definitely agree with @eiriktsarpalis then they say Default has a very specific meaning and should probably be avoided here. Here's my original justification:

I had first proposed an overload for FirstOrDefault() which allows you to specify the "default" value it returns, however tannergooding suggested it might be better to create an entirely new method on the grounds that the signatures may be too similar. I decided on the OrElse suffix because it has a history in other languages. This gives it the advantage of being well-known. It should pair rather nicely with its existing OrDefault counterpart, on the grounds of them sharing similar yet distinct functions.

@Foxtrek64 I agree with you, OrElse is a better suffix than OrGiven. Let's make it part of the official proposal.

@terrajobst what is the recommended way of updating the proposal? Should I edit the issue description or post an updated comment?

Video

Looks good as proposed, but there was a concern raised that the Queryable methods might have an impact on Linq-to-SQL, which would be good to verify (@ajcvickers do you have insight or contacts here?).

```C#
namespace System.Linq
{
public static partial class Enumerable
{
public static TSource SingleOrDefault(this IEnumerable source, TSource defaultValue);
public static TSource SingleOrDefault(this IEnumerable source, Func predicate, TSource defaultValue);

    public static TSource FirstOrDefault<TSource>(this IEnumerable<TSource> source, TSource defaultValue);
    public static TSource FirstOrDefault<TSource>(this IEnumerable<TSource> source, Func<TSource, bool> predicate, TSource defaultValue);

    public static TSource LastOrDefault<TSource>(this IEnumerable<TSource> source, TSource defaultValue);
    public static TSource LastOrDefault<TSource>(this IEnumerable<TSource> source, Func<TSource, bool> predicate, TSource defaultValue);
}

public static class Queryable
{
    public static TSource SingleOrDefault<TSource>(this IQueryable<TSource> source, TSource defaultValue);
    public static TSource SingleOrDefault<TSource>(this IQueryable<TSource> source, Expression<Func<TSource, bool>> predicate, TSource defaultValue);

    public static TSource FirstOrDefault<TSource>(this IQueryable<TSource> source, TSource defaultValue);
    public static TSource FirstOrDefault<TSource>(this IQueryable<TSource> source, Expression<Func<TSource, bool>> predicate, TSource defaultValue);

    public static TSource LastOrDefault<TSource>(this IQueryable<TSource> source, TSource defaultValue);
    public static TSource LastOrDefault<TSource>(this IQueryable<TSource> source, Expression<Func<TSource, bool>> predicate, TSource defaultValue);
}

}
```

@bartonjs This should've been caught (much) earlier, but there's one tiny change that needs to happen to these additions: The overloads in Queryable that take in Func<TSource, bool> predicate needs to be Expression<Func<TSource, bool>> predicate, otherwise Query-providers can't do their magic.

@Joe4evr Good catch. Since they were approved as "add TSource defaultValue to existing overloads" I've gone ahead and fixed the approved shape to be sensible.

nit: Perhaps Given would be a better name than Default?

To add my $0.02, I had suggested Else in #33443 for similar reasons you state. However, the OrElse paradigm has a history in other languages and is what I would expect for these endpoints. FirstOrGiven seems rather alien in that sense.

Ultimately I feel either would be fine, but I definitely agree with @eiriktsarpalis then they say Default has a very specific meaning and should probably be avoided here. Here's my original justification:

I had first proposed an overload for FirstOrDefault() which allows you to specify the "default" value it returns, however tannergooding suggested it might be better to create an entirely new method on the grounds that the signatures may be too similar. I decided on the OrElse suffix because it has a history in other languages. This gives it the advantage of being well-known. It should pair rather nicely with its existing OrDefault counterpart, on the grounds of them sharing similar yet distinct functions.

@bartonjs I'm not sure if it's too late for this seeing as it's already been through review, but I would like to voice a strong disapproval for the MethodOrDefault() overload/signature in these methods for the reasons above. I'm not sure how the review process works here, but if it's at all possible I would like to see MethodOrElse() be the preferred signature.

If this change is approved, I would be happy to volunteer to implement this.

Since this enhancement is "I want the existing *OrDefault method, but to change what it thinks the default is", it seems like a natural overload to me, rather than having SingleOrDefault() and SingleOrElse(TSource elseValue).

Additionally, .NET has had the "OrDefault" suffix for longer than C# has had the default keyword, so there's precedent for that name (OK, largely from Enumerable and Queryable, but also Nullable and dictionaries). And "OrElse" sounds pretty aggressive to me... "give me the first item, or else!" :smile:.

I definitely don't see changing these members to OrElse passing review, and am skeptical that we'd use that suffix on new functionality on Enumerable or Queryable... or even entirely new types.

If this change is approved, I would be happy to volunteer to implement this.

Thanks for volunteering @Foxtrek64, I have assigned the issue to you.

@Foxtrek64 For reference, a similar method that has existed for ages in .NET is Nullable<T>.GetValueOrDefault(T defaultValue).

You could use the operator "??" wich is great because :

  • More readable when used to it
  • More flexible (Allow throw, other linq, ...)
  • More performent since it doesn't need to allocate the default
  • Doesn't need new method and use the current logic
  • Work with Queryable

For any case where you have a struc like dictionary, int, datetime i use two trick depending of the case

  • When you want a nullable property in struct like KeyValuePair i just do '?.Key ?? throw new XXX;'.
  • Datetime and int without rule i juste move the rule in a where 'Where(x => x == z).OffType().FirstOrDefault()'. This way at most only one is encapsulate in Nullable<>

I used to implement this overload for one reason. When you read "or default" i expect my first param to be default. With "??" the default is after the predicate wich can be long in some case and become hard to read.

```C#
const string MostPopularName = "Dotnet";
string foo = nameRows.FirstOrDefault(MostPopularName, nameRow =>
nameRow.State == MyEnumOfState.MyState &&
!bannedWords.Contains(nameRow) &&
someOtherRules);

```C#
const string MostPopularName = "Dotnet";
string foo = nameRows.FirstOrDefault(nameRow =>
                        nameRow.State == MyEnumOfState.MyState &&
                        !bannedWords.Contains(nameRow) &&
                        someOtherRules, 
                        MostPopularName);

C# const string MostPopularName = "Dotnet"; string foo = nameRows.FirstOrDefault(nameRow => nameRow.State == MyEnumOfState.MyState && !bannedWords.Contains(nameRow.Name) && someOtherRules) ?.Name ?? MostPopularName;

A few questions-

System.Collections.Immutable has its own LINQ library. Should I extend this, or any additional libraries, in this change?

Should we add ElementAtOrDefault<TSource>(this IEnumerable<TSource> source, int index, TSource other); to this change? It was not included in the original scope but I feel it is likely a logical addition.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

jzabroski picture jzabroski  路  3Comments

chunseoklee picture chunseoklee  路  3Comments

yahorsi picture yahorsi  路  3Comments

omajid picture omajid  路  3Comments

btecu picture btecu  路  3Comments