Add TrySingle<TSource>(IEnumerable<TSource>, out TSource)

[shmuelie]

Motivation

In LINQ we have the Single and SingleOrDefault methods, but they don't tell you if it succeeded or not.

The problem with these methods is that if there is more than one element in source (or more than one matching element in case of predicate) they all throw a System.InvalidOperationException. That is fine if more than one element is an error or very rare. If more than one element is common or expected then this will cause major slow downs.

Additionaly, in cases of (example: int collections), default(int) is 0, which wouldn't tell the caller whether the operation succeeded (returning the item 0) or not (returning the default 0).

Proposal

namespace System.Linq
{
    public static class Enumerable
    {
        public static TSource Single<TSource>(this IEnumerable<TSource> source);
        public static TSource Single<TSource>(this IEnumerable<TSource> source, Func<TSource, bool> predicate);
        public static TSource SingleOrDefault<TSource>(this IEnumerable<TSource> source);
        public static TSource SingleOrDefault<TSource>(this IEnumerable<TSource> source, Func<TSource, bool> predicate);
+       public static bool TrySingle<TSource>(this IEnumerable<TSource> source, out TSource element);
+       public static bool TrySingle<TSource>(this IEnumerable<TSource> source, Func<TSource, bool> predicate, out TSource element);

        public static TSource First<TSource>(this IEnumerable<TSource> source);
        public static TSource First<TSource>(this IEnumerable<TSource> source, Func<TSource, bool> predicate);
        public static TSource FirstOrDefault<TSource>(this IEnumerable<TSource> source);
        public static TSource FirstOrDefault<TSource>(this IEnumerable<TSource> source, Func<TSource, bool> predicate);
+       public static bool TryFirst<TSource>(this IEnumerable<TSource> source, out TSource element);
+       public static bool TryFirst<TSource>(this IEnumerable<TSource> source, Func<TSource, bool> predicate, out TSource element);

        public static TSource Last<TSource>(this IEnumerable<TSource> source);
        public static TSource Last<TSource>(this IEnumerable<TSource> source, Func<TSource, bool> predicate);
        public static TSource LastOrDefault<TSource>(this IEnumerable<TSource> source);
        public static TSource LastOrDefault<TSource>(this IEnumerable<TSource> source, Func<TSource, bool> predicate);
+       public static bool TryLast<TSource>(this IEnumerable<TSource> source, out TSource element);
+       public static bool TryLast<TSource>(this IEnumerable<TSource> source, Func<TSource, bool> predicate, out TSource element);

        public static TSource ElementAt<TSource>(this IEnumerable<TSource> source, int index);
        public static TSource ElementAtOrDefault<TSource>(this IEnumerable<TSource> source, int index);
+       public static bool TryElementAt<TSource>(this IEnumerable<TSource> source, int index, out TSource element);
    }

    public static class Queryable
    {
        public static TSource Single<TSource>(this IQueryable<TSource> source);
        public static TSource Single<TSource>(this IQueryable<TSource> source, Expression<Func<TSource, bool>> predicate);
        public static TSource SingleOrDefault<TSource>(this IQueryable<TSource> source);
        public static TSource SingleOrDefault<TSource>(this IQueryable<TSource> source, Expression<Func<TSource, bool>> predicate);
+       public static (bool success, T value) TrySingle<T>(this IQueryable<T> source);
+       public static (bool success, T value) TrySingle<T>(this IQueryable<T> source, Expression<Func<TSource, bool>> predicate);
+       public static bool TrySingle<TSource>(this IQueryable<TSource> source, out TSource element);
+       public static bool TrySingle<TSource>(this IQueryable<TSource> source, Expression<Func<TSource, bool>> predicate, out TSource element);

        public static TSource First<TSource>(this IQueryable<TSource> source);
        public static TSource First<TSource>(this IQueryable<TSource> source, Expression<Func<TSource, bool>> predicate);
        public static TSource FirstOrDefault<TSource>(this IQueryable<TSource> source);
        public static TSource FirstOrDefault<TSource>(this IQueryable<TSource> source, Expression<Func<TSource, bool>> predicate);
+       public static (bool success, T value) TryFirst<T>(this IQueryable<T> source);
+       public static (bool success, T value) TryFirst<T>(this IQueryable<T> source, Expression<Func<TSource, bool>> predicate);
+       public static bool TryFirst<TSource>(this IQueryable<TSource> source, out TSource element);
+       public static bool TryFirst<TSource>(this IQueryable<TSource> source, Expression<Func<TSource, bool>> predicate, out TSource element);

        public static TSource Last<TSource>(this IQueryable<TSource> source);
        public static TSource Last<TSource>(this IQueryable<TSource> source, Expression<Func<TSource, bool>> predicate);
        public static TSource LastOrDefault<TSource>(this IQueryable<TSource> source);
        public static TSource LastOrDefault<TSource>(this IQueryable<TSource> source, Expression<Func<TSource, bool>> predicate);
+       public static (bool success, T value) TryLast<T>(this IQueryable<T> source);
+       public static (bool success, T value) TryLast<T>(this IQueryable<T> source, Expression<Func<TSource, bool>> predicate);
+       public static bool TryLast<TSource>(this IQueryable<TSource> source, out TSource element);
+       public static bool TryLast<TSource>(this IQueryable<TSource> source, Expression<Func<TSource, bool>> predicate, out TSource element);

        public static TSource ElementAt<TSource>(this IQueryable<TSource> source, int index);
        public static TSource ElementAtOrDefault<TSource>(this IQueryable<TSource> source, int index);
+       public static (bool success, T value) TryElementAt<T>(this IQueryable<T> source, int index);
+       public static bool TryElementAt<TSource>(this IQueryable<TSource> source, int index, out TSource element);
    }
}

Original

I'd like to propose a new LINQ method: TrySingle. The idea is pretty much the same as the existing Single method except it does not throw an exception when there is not only a single item in the sequence.

To do this I made to modifications to the Single contract:

1. First the method returns bool. true if the sequence contains exactly one item, false otherwise.
2. I added an out parameter of type TSource. This will "hold" the single element on success. On failure it is filled with default(TSource).

I have found this method very useful in my own code and if accepted can quickly put together a pull request.

[JonHanna]

If one was going to do this, I would see it as cleaner if all of the methods with XxxOrDefault() had a TryXXX.

[shmuelie]

I do see what you're getting at but I'm not "replacing" the SingleOrDefault() method, since that still throws an exception.

[MgSam]

I'd say any API additions with the Try pattern should be held off until the Tuple design for C# 7 is finalized. It might be better for these types of methods to return tuples.

[shmuelie]

I actually like the way this works because it makes using this method in a if statement very easy:

int I;
if (seq.TrySingle(out I))
{
   ...
}

(I admit though that TryXXX might not be the best name for this method though)

[JonHanna]

I wouldn't suggest any of them as a replacement (I'd argue vehemently against), but rather just as one might say "I want to get the single matching element here, or if there isn't a single matching element, know that I failed to get it" one might also say "I want to get the first matching element here, or if there isn't one, know that I failed to get it" and so on.

FWIW, some code I'm playing with now has a TryGetFirst, TryGetLast and TryGetElementAt because it became necessary to distinguish from default(TSource) of a failure and of the element happening to be the default.

[shmuelie]

Ah yes, for Last, First, and ElementAt a TryXXX method would be great. The issue here is that I'm trying to avoid an exception which event the xxxOrDefault doesn't do. My use of the TryXXX naming may not have been the best choice.

[JonHanna]

Yep. Much of the time the SingleOrDefault would work, but if the default value is a possible real value, then it's no good to you.

[shmuelie]

How would SingleOrDefault work? My case is that having more than one item is common and that means both Single and SingleOrDefault throw an exception, something I really don't want happening! (SingleOrDefault returns default on no items)

[JonHanna]

Ah. I get your meaning now. Single of course has two failure modes...

[shmuelie]

Maybe a name other than TrySingle?

[JonHanna]

I wonder if a caller might want to distinguish failure because there are none from failure because there are more than one?

[shmuelie]

So say return an enum?

enum SequenceLength
{
   Empty,
   Single,
   More
}

[shmuelie]

After the discussion above, some more thinking myself, and looking at the contributing guidelines...

Rational

In LINQ we have the Single and SingleOrDefault methods:

namespace System.Linq
{
    public static class Enumerable
    {
        public static TSource Single<TSource>(this IEnumerable<TSource> source);
        public static TSource Single<TSource>(this IEnumerable<TSource> source, Func<TSource, bool> predicate);
        public static TSource SingleOrDefault<TSource>(this IEnumerable<TSource> source);
        public static TSource SingleOrDefault<TSource>(this IEnumerable<TSource> source, Func<TSource, bool> predicate);
    }
}

The problem with these methods is that if there is more than one element in source (or more than one matching element in case of predicate) they all throw a System.InvalidOperationException. That is fine if more than one element is an error or very rare. If more than one element is common or expected then this will cause major slow downs.

New Methods

I propose the following methods be added:

namespace System.Linq
{
    public static class Enumerable
    {
        public static bool Single<TSource>(this IEnumerable<TSource> source, out TSource element);
        public static bool Single<TSource>(this IEnumerable<TSource> source, Func<TSource, bool> predicate, out TSource element);
    }
}

Contracts

public static bool Single<TSource>(this IEnumerable<TSource> source, out TSource element);

1) If source is null, throw System.ArgumentNullException. 2) If source is empty or has more than one element, set element to default(TSource) and return false. 3) If source has one element, set element to that element and return true.

public static bool Single<TSource>(this IEnumerable<TSource> source, Func<TSource, bool> predicate, out TSource element);

1) If source or predicate is null, throw System.ArgumentNullException. 2) If source has no elements or has no elements that predicate returns true for or has more than one element that predicate returns true for, set element to default(TSource) and return false. 3) If source has one element that predicate returns true for, set element to that element and return true.

---

As to @JonHanna 's point about wanting to know more information, I feel that would be a whole new method.

[Clockwork-Muse]

...if more than one element is common/expected, then you have one of two situations:

1. You don't particularly care which you get, so First(...) would be appropriate.
2. You do care, so the exception thrown in Single(...) is appropriate.

"Exceptions are slow" is irrelevant in the case of requesting user input to resolve the issue - displaying to the screen and acting on the input is going to take far longer.

If you're planning on doing something like showing a pop-up with multiple results (if they exist) for selection, you're going to have to get them regardless. At that point, it'd be better to start with the results of a Where(...) query and go from there.

What was the situation you were planning on using this method to solve?

[shmuelie]

My use case is a user is changing selection. They can:

• Have no items selected: I do nothing.
• Have one item selected: I process the selected item.
• Have more than one item selected: I do nothing.

[Clockwork-Muse]

Hmm, wouldn't that be approached as:

• Your processing is the only action possible: it is an error condition to select more than one item.
• There are more possible actions: it is an error condition to active your processing.

I mean, in the case where the user has multiple items selected and attempts your processing, you can't actually do nothing; you need to at least let them know you can't continue. For doing things like greying out UI buttons based on number of selected items, you probably want to be using Count(...).

[shmuelie]

The processing is automatic and there are multiple "handlers" for selection. Some handle more than one item, this one doesn't. So I do silently do nothing. I could use Count but I don't care beyond 2, so it would be wasting cycles counting every item

[karelz]

@VSadov says this is useful addition. Without reading it all, we think TrySingle is better name. Updating proposal on the top.

[shmuelie]

Should I create a PR based on my implementation?

[karelz]

Not yet, we need to do API review (scheduled every Tuesday at 10am PT). We will likely require all Try* APIs for completness (final decision hopefully tomorrow).

[karelz]

We'll mark it up for grabs and assign it to you @SamuelEnglard when it is ready for implementation.

[shmuelie]

Makes sense. And easy enough to do.

[OmarTawfik]

@karelz updated the proposal.

[jnm2]

I prefer to solve this with T? SingleOrNull<T>(this IEnumerable<T> source) where T : struct. I have SingleOrNull, FirstOrNull, MaxOrNull, SumOrNull, IReadOnlyDictionary.GetValueOrNull, etc.

They are all shorthand for .AsNullable().SingleOrDefault(), etc. This itself is shorthand for .Select(_ => (T?)_).SingleOrDefault().

These are all for value types. For ref types, *OrNull becomes *OrDefault.

[karelz]

@jnm2 are you proposing changes to the API shape? Or just mentioning an alternative in general? Personally I think the Try pattern is already well established in BCL and easier to use / comprehend. Also, `OrDeaultdoes not work well with cases whennull` is acceptable value.

[jnm2]

@karelz Just making sure the concept gets discussed before the API is finalized. As you pointed out, it doesn't help in cases where null is acceptable. On that basis I don't think the API shape should be changed.

[karelz]

Approved as is in the top most post.

[jamesqo]

@karelz I just got cc'd from @AlexRadch's PR at https://github.com/dotnet/corefx/pull/14276. I know this is the second time I've done this today, but can the naming of this API be reconsidered?

I wholly understand and agree with adding this API. However, as a contributor to Linq, I feel rather strongly that TryGetSingle would be a better name. Virtually all of the Try* methods-- TryGetValue, TryAdd, etc-- have a verb after the Try. This translates in my mind as "try getting this value/adding this item, and return whether we succeeded." TrySingle is a verb + a noun, which does not translate smoothly in my mind at all. Same for TryFirst, TryLast, and TryElementAt. I strongly suggest that the naming be reconsidered, even if it is 3 extra characters. @SamuelEnglard and others, do you disagree?

[JonHanna]

I agree on Add. The ideal would be if there was an appropriate adjective to match most of the similar methods, but there isn't. The To… form isn't appropriate, so really TryGet… seems the best option here.

[AlexRadch]

@karelz I can rename these methods to TryGet. Is it approved?

[JonHanna]

These also make sense to include on Queryable.

[shmuelie]

I do agree with @jamesqo point, though I think the missing "Get" is really with the original methods to start with.

[JonHanna]

Well, the descriptive quality of the adjectives makes sense on their own, but when the verb Try is introduced they become stranger. Or at least now that @jamesqo pointed it out they seem strange!

On the Queryable matter, we're keeping the parity with SkipLast and TakeLast and perhaps (still being discussed) restoring it for Prepend and Append. I think it definitely makes sense here too.

[AlexRadch]

As I can see Queryable does not have any methods with out parameter. Is it possible to have such methods in Queryable?

[karelz]

We already discussed TryGetFirst vs. TryFirst naming - see https://github.com/dotnet/corefx/issues/6073#issuecomment-263421263. I think of it more as pattern of adding Try prefix before the method name.

Queryable companion methods sound ok to me - @OmarTawfik can you please confirm? (@VSadov is OOF for another week I think).

[karelz]

@SamuelEnglard I have to apologize - I didn't follow up on my promise of assigning the issue to you (https://github.com/dotnet/corefx/issues/6073#issuecomment-263437700). It was by accident.

@AlexRadch that's another reason why it would be great if you can please ping each issue before you grab it in future. It will be easier to organize who works on what - avoiding duplicated efforts, "stolen features", etc. Thank you!

[AlexRadch]

@karelz sorry. I did not see this comment.

[karelz]

That makes two of us ;-) -- I think it is as a good motivation to ping the issue in future, before you start working on it :)

[shmuelie]

@karelz it's cool. I've literally have the code sitting on my machine, just haven't had the time to "move" it to my fork to push up so no time lost on my end

We already discussed TryGetFirst vs. TryFirst naming - see dotnet/corefx#6073 (comment). I think of it more as pattern of adding Try prefix before the method name.

We didn't really discuss it as much as close over it.

[jamesqo]

We didn't really discuss it as much as close over it.

:+1:

@karelz, to give you a better sense of how I am thinking about this, it is like in English saying "try train" instead of "try catch train" or "try car" instead of "try stop car". I think it would make much more sense with a verb.

[jnm2]

Grammatically, I agree that TryGetSingle is best. With TrySingle, You're not "trying to single" (single as a verb can only be used transitively) and you're not "trying a single" (the trying is being applied to the sequence, not to the result).

You can sacrifice grammatical sense for terseness, but in this case it just sounds awkward.

Also we have TryGetValue on dictionaries. We should stick with existing standards.

[shmuelie]

Using current spec but WIP https://github.com/SamuelEnglard/corefx/blob/TrySingle-Issue6073/src/System.Linq/src/System/Linq/Single.cs

[karelz]

We didn't really discuss it as much as close over it.

What I meant we discussed it briefly in triage with @VSadov and @OmarTawfik. It was raised here (https://github.com/dotnet/corefx/issues/6073#issuecomment-183438071) by @JonHanna, but it didn't spark deeper discussion here. Moreover, in the API review (with English native speakers), no one raised it either.

I personally believe that having the name closer to Single method name (BTW: was it a mistake of the past to name it GetSingle in the first place?) is better for discoverability of the API and to create connection between the 2 APIs. But to be honest, I don't care too much what we choose + I am ESL, so I wouldn't push too hard for my opinion anyway ;-).

@terrajobst @danmosemsft @bartonjs @KrzysztofCwalina @stephentoub @weshaggard can you please vote on the name choice TrySingle vs. TryGetSingle? Do we have guidelines? Do we care? (Proper English grammar TryGetSingle vs. using Try as prefix to existing method names - TrySingle)

[stephentoub]

can you please vote on the name choice TrySingle vs. TryGetSingle?

If Single were instead GetSingle and SingleOrDefault were instead GetSingleOrDefault, then TryGetSingle would make sense. But they're not. The Try pattern is generally to simply prefix the operation with "Try" and have it return a bool with the actual result as an out parameter, so it seems we should do that here as well. Thus I vote for TrySingle. e.TrySingle is no more strange than e.Single, and there's value in having them be consistent with each other, more so I believe than breaking from the pattern and trying to clarify the name.

[bartonjs]

I concur with Steve. The original mistake was having a method with no verb, but we should stick to the naming scheme of just prepending "Try". Because the breakdown is "try that other method... return false if it would have thrown an exception had I called it". Aka

try { ret = Single(); return true; } catch { return false; }

(And, yeah, Karel, I'm agreeing with you, too, but you mentioned both sides and asked a question, Steve only answered :smile:)

[jamesqo]

@stephentoub @bartonjs I generally think of Linq's extension methods as properties rather than methods. First() in my mind is not performing an action, but retrieving a trivially obtainable aspect of the enumerable. However, with Try the operation is no longer trivial because it can potentially fail. So my mind converts Single (an aspect) -> GetSingle() (an action) -> TryGetSingle() (an action that might fail).

Having said that, I do not want to block this proposal since there is already a PR for it. Plus @bartonjs' comment clarifies things a bit more. I guess having TryX would be OK.

[shmuelie]

@jamesqo all the methods we're adding TryX to had the potential to fail in the first place.

[jamesqo]

@karelz Can this be API reviewed again tomorrow for the corresponding methods to be added to Queryable? All of the TryFirst, TryLast, TryElementAt, and TrySingle overloads mentioned in the description?

[AlexRadch]

@jamesqo There is no solution how to add TryX methods to Queryable.

[jamesqo]

@AlexRadch I only glanced at his comments, but I think @bartdesmet mentioned some possible solutions at https://github.com/dotnet/corefx/pull/14276#discussion_r91574861? It seems like just he's waiting for input from @divega about which is the best method.