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

WhereNotNull operator #1262

Open
Noggog opened this issue Jul 30, 2020 · 9 comments
Open

WhereNotNull operator #1262

Noggog opened this issue Jul 30, 2020 · 9 comments

Comments

@Noggog
Copy link

Noggog commented Jul 30, 2020

Feature request

Which next library version (i.e., patch, minor or major)?

Minor, I suppose

How commonly is this feature needed (one project, several projects, company-wide, global)?

Global

Please describe the feature.

C#8 introduced nullability compiler features, which is slowly permeating codebases as people upgrade. One of the common clashes I've seen with Rx patterns, though, is the WhereNotNull style logic. This is a typical code snippet highlighting the friction:

observable.Where(x => x != null).Subscribe(x => x!.DoStuff())

Here, the user checks that an item is not null, but the following Rx operator is unaware that the item is now null safe, and so has to add an ! operator to "force it through".

In my own codebases, I have cleaned up this use case by adding the following extension:

public static IObservable<T> WhereNotNull<T>(this IObservable<T?> source)
    where T : class
{
    return source
        .Where(u => u != null)
        .Select(u => u!);
}

A note on the fundamental concept: This operator takes in a nullable T?, but returns a non-nullable T type. This cleans up downstream operators that can now be certain the item is not null:

observable.WhereNotNull().Subscribe(x => x.DoStuff())

However, it has been noted that this WhereNotNull route introduces an extra Select statement that the original did not have. This performance bite results in some people continuing to use the initial code snippets mentioned.

Overall, this seems like a fundamental concept to consider given the introduction of nullability concepts. I'm hoping the Rx codebase might consider adding a dedicated official operator that could do the T? -> T shift in one fell swoop, rather than leveraging two chained operators like my extension method does.

Essentially, it would be an alternative of Where, except the input generic would be nullable T?, while the output generic would be non-nullable T. The logic would do the null check, and produce signals where the input was not null, while migrating it to the non-null generic space for downstream operators.

I was going to dip into producing a PR to introduce this, but I wanted to open a discussion before putting in the work to confirm it's desirable in the first place.

Thanks for the consideration!

@quinmars
Copy link
Contributor

I'd wait for System.Linq, if it is introduced there it should also be introduced for System.Reactive.

I think you can use instead:

public static IObservable<T> WhereNotNull<T>(this IObservable<T?> source)
    where T : class
{
    return source.Where(u => u != null)!;
}

This will introduce no (or very little) overhead.

@apobekiaris
Copy link

or a more abstracted method like

public static IObservable<TSource> WhenNotDefault<TSource,TValue>(this IObservable<TSource> source,Func<TSource,TValue> valueSelector) =>source
	        .Where(source1 => !valueSelector(source1).IsDefaultValue());

public static IObservable<TSource> WhenNotDefault<TSource>(this IObservable<TSource> source) => source.Where(_ => !_.IsDefaultValue());

		
public static bool IsDefaultValue<TSource>(this TSource source){
	var def = default(TSource);
	return def != null ? def.Equals(source) : source == null;
}

@quinmars
Copy link
Contributor

@apobekiaris I do not see how this solves the NRT problem mentioned by the OP.

@apobekiaris
Copy link

apobekiaris commented Jul 30, 2020

it emits when value is not the default(T) and if T is reference type it emits when not null, to me it looks like your WhereNotNull emits the same

@quinmars
Copy link
Contributor

Ah, the main point of that operator is, that it converts a nullable reference type to a non-nullable reference type (T? -> T). The question mark in the source type is quickly overlooked.

@apobekiaris
Copy link

dump apologies thought I was helping

@Noggog
Copy link
Author

Noggog commented Jul 31, 2020

Hey all! Thanks for the discussion and suggestions.

The initial suggestion seems like a winner to me for the short term. I was not aware you could apply the ! operator in that fashion, but it makes sense in hindsight.

I will be upgrading my setup to use that, but I still do think Linq in general might benefit from having that operator built in to facilitate the new nullability featuresets for the masses.

Feel free to close this if you want to, as it seems to be getting out of scope of just Rx, and towards a larger discussion of Linq specifically.

@quinmars
Copy link
Contributor

For reference, here is the corresponding LINQ issue dotnet/runtime#30381.

@bartdesmet
Copy link
Collaborator

We'll await the resolution on the LINQ to Objects side. Once we add something over here in Rx, we can't take it away (compatibility), and it'd be unfortunate to end up with something that's different from whatever may or may not come in LINQ to Objects.

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

4 participants