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

Filter out nulls given a T? (struct) sequence, returning just T values #609

Closed
fowl2 opened this issue Aug 28, 2019 · 5 comments
Closed

Comments

@fowl2
Copy link

fowl2 commented Aug 28, 2019

public static IEnumerable<T> WhereHasValue<T>(this IEnumerable<T?> enumerable)
    where T : struct
{
    foreach (var item in enumerable)
        if (item.HasValue)
           yield return item.Value;
}

This is better than just writng Where(a => a.HasValue) because it returns T instead of T?.

@atifaziz
Copy link
Member

There's already Choose that provides the same functionality but in a more generic way. You can write WhereHasValue in terms of Choose as a one-liner:

public static IEnumerable<T> WhereHasValue<T>(this IEnumerable<T?> source)
    where T : struct =>
    source.Choose(e => e is T v ? (true, v) : default);

I believe that if you start using Choose, you'll find it's more powerful because it gives you more flexibility and leads to more generic code. For example, instead of introducing WhereHasValue, what if you had the following way of deconstructing or re-shaping a T? into (bool, T)?

static partial class Nullable
{
    public static (bool HasValue, T Value) Deconstruct<T>(T? nullable) where T : struct =>
    nullable.HasValue ? (true, nullable.Value) : default;
}

This is more general and independent of whether T? is contained in a sequence or not. Then you can combine and use it with Choose as follows:

"O,l,2,3,4,S,6,7,B,9".Split(',')
    .Select(s => int.TryParse(s, out int n) ? (int?)n : null)
    .Select(Nullable.Deconstruct)
    .Choose(x => x)
    .ForEach(Console.WriteLine); // prints 2, 3, 4, 6, 7 & 9 on separate lines

The above example is somewhat artificial because I'd write it simply as:

"O,l,2,3,4,S,6,7,B,9".Split(',')
    .Select(s => int.TryParse(s, out int n) ? (true, n) : default)
    .Choose(x => x)
    .ForEach(Console.WriteLine);

but my point is that if you have a sequence of T? (say you don't have a choice because you called a third-party function that handed it to you that way), you can project via Nullable.Deconstruct and then pipe to Choose to get the desired effect without having to define WhereHasValue.

By the way, if you use my Optuple library, then you get Nullable.Deconstruct for free, except being an extension method, it's called Option.ToOption there. So using it, you could write:

"O,l,2,3,4,S,6,7,B,9".Split(',')
    .Select(s => int.TryParse(s, out int n) ? (int?)n : null)
    .Select(Option.ToOption)
    .Choose(x => x)
    .ForEach(Console.WriteLine);

or just:

"O,l,2,3,4,S,6,7,B,9".Split(',')
    .Select(s => int.TryParse(s, out int n) ? Some(n) : default)
    .Choose(x => x)
    .ForEach(Console.WriteLine);

Optuple is non-viral as it introduces or imposes no new data type that leaks into your API.

If you're interested in the discussion surrounding the design of Choose then see #358.

I hope the above demonstrates that WhereHasValue isn't strictly needed and while we generally have discouraged adding one-liner helpers to MoreLINQ (preferring they be added locally to projects needing them for clarity), I think nulls are prevalent enough in .NET to consider adding the following:

public static IEnumerable<T> NonNull<T>(this IEnumerable<T?> source)
    where T : struct =>
    source.Choose(e => e switch { null => default, T v => (true, v) });

public static IEnumerable<T> NonNull<T>(this IEnumerable<T> source) =>
    source.Choose(e => e switch { null => default, T v => (true, v) });

I'd like to get the thoughts of @fsateler and @leandromoh on this.

I am hesitant to add the second overload because the signature doesn't do justice now that we can express nullable reference types since C# 8 because technically speaking, you'd like to say that T is nullable on input but never null on output and such a constraint cannot exist. You would have to introduce a new parameterised type like TValue and a projection such that you can apply the new (in preview 7) notnull constraint to TValue:

public static IEnumerable<TValue>
    NonNull<T, TValue>(
        this IEnumerable<T> source,
        Func<T, TValue> valueSelector)
        where TValue : notnull =>
    source.Choose(e => e switch { null => default, T v => (true, valueSelector(v)) });

Even with the added constraint, valueSelector cannot say that its T argument is never going to be null and so each call to NotNull would always be NonNull(x => x!). In fact, I would even add the class constraint on T because it won't make sense for nullable value types.

public static IEnumerable<TValue>
    NonNull<T, TValue>(
        this IEnumerable<T> source,
        Func<T, TValue> valueSelector)
        where T : class
        where TValue : notnull =>
    source.Choose(e => e switch { null => default, T v => (true, valueSelector(v)) });

Anyway, this is all odd enough to simply avoid overloading.

@atifaziz atifaziz changed the title WhereHasValue - Nullable value types Filter out nulls given a T? (struct) sequence, returning just T values Aug 28, 2019
@leandromoh
Copy link
Collaborator

@atifaziz since this method is as simple as Where(a => a.HasValue).Cast<T>() I dont think it is worth to adds it on morelinq given the effort (debate implementation, documentation, tests, and review for each of these).

@atifaziz
Copy link
Member

I think we can close this solely on the basis that there's been no follow-up from @fowl2 so this issue can be considered stale.

@fowl2
Copy link
Author

fowl2 commented Oct 14, 2019

Hi @atifaziz, what follow-up were you expecting? My understanding this was waiting until the c# 8.0's nullability annotations were released so that it could be done as well as possible with respect to that.

PS. Thanks again for this work!

@atifaziz
Copy link
Member

@fowl2 Perhaps the right word was reaction, rather than follow-up, to the ideas and thoughts I laid out. For one, if Choose is good enough then do we really need this?

My understanding this was waiting until the c# 8.0's nullability annotations were released so that it could be done as well as possible with respect to that.

As I said, to do this right would make it ugly unless someone else has a better idea. If you're going to need an artificial projection, you might as well go with Choose.

I was also waiting to hear from @leandromoh and @fsateler, and while @leandromoh has spoken his mind and voted that this is not worth the trouble, I am guessing that @fsateler is just occupied.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants