ReactiveX / RxJava

RxJava – Reactive Extensions for the JVM – a library for composing asynchronous and event-based programs using observable sequences for the Java VM.
Apache License 2.0
47.85k stars 7.61k forks source link

2.x: Handling null values #4644

Closed paveldudka closed 7 years ago

paveldudka commented 8 years ago

With the upcoming RxJava2 release one of the important changes is that null is no longer accepted as a stream element.

Honestly, I have mixed feelings about this change and part of me understands that it will enforce clean APIs, but I can see a number of use cases when this might be a problem.

For instance, in my app I have an in-memory cache:

@Nullable CacheItem findCacheItem(long id);

CacheItem might not be present in cache, so method might return null value.

The way it is used with Rx* - is as following:

Observable<CacheItem> getStream(final long id) {
    return Observable.fromCallable(new Callable<CacheItem>() {
        @Override public CacheItem call() throws Exception {
            return findCacheItem(id);
        }
    });
}

So with this approach, I might get null in my stream which is totally valid situation, so it is handled properly on receiving side - let's say UI changes its state if item is not present in cache:

Observable.just(user)
          .map(user -> user.getName())
          .map(name -> convertNameToId(name))
          .flatMap(id -> getStream(id))
          .map(cacheItem -> getUserInfoFromCacheItem(cacheItem))
          .subscribe(
              userInfo -> {
                  if(userInfo != null) showUserInfo();
                  else showPrompt();
              }
          );

With RxJava2 I am no longer allowed to post null down the stream, so I either need to wrap my CacheItem into some other class and make my stream produce that wrapper instead or make quite big architectural changes.

Wrapping every single stream element into nullable counterpart doesn't look right to me.

Am I missing something fundamental here?

It seems like the situation like mine is quite popular, so Im curious what is the recommended strategy to tackle this problem given new "no null" policy in RxJava2?

artem-zinnatullin commented 8 years ago

https://github.com/ReactiveX/RxJava/wiki/What's-different-in-2.0#nulls

vietj commented 7 years ago

I believe this is a valid case and the link pointed by @artem-zinnatullin does not address the situation.

In a language supporting union types (like Ceylon) that would be valid and the case above would be typed as Observable<CacheItem | Missing>.

As @paveldudka said the only way to handle this without loosing type safety with the current situation would be to use a wrapper type:

for instance

Observable<Optional<CacheItem>> getStream(final long id) { ... }

In this specific case maybe a more convenient way would be to use a pair as this is a cache lookup, so I would likely use something like:

Observable<Map.Entry<Long, CacheItem> getStream(final long id) { ... }

But this is sometimes not feasible.

akarnokd commented 7 years ago

Alternative is to use a null-sentinel value of CacheItem and do a comparison against it instead of null:

static final CacheItem NOT_FOUND = new CacheItem(...);

Observable.just(1)
.map(v -> {
   CacheItem ci = getFromCache(v);
   return ci != null ? ci : NOT_FOUND;
})
.filter(ci -> ci != NOT_FOUND)
...
vietj commented 7 years ago

@akarnokd if you create a reactive stream with objects you do not control this can be a problem (for example you create a Flowable of objects of a library that keeps control over the instance creation), using then concatMap does the same (but is more expensive)

paveldudka commented 7 years ago

I believe the most "natural" way for dealing with null values in RxJava2 would be to return stream of Optional elements instead or "raw" ones.

danielesegato commented 7 years ago

This is really bad.

Nulls values are really needed in Java. You are forcing to clutter the API of our apps with Observable<Optional<Stuff>> when we could just use Observable<Stuff>.

I also don't like at all the NOT_FOUND suggestion from @akarnokd and I'm specially concerned of this when you need to wrap others APIs (where you have no control on).

I can understand the decision about nulls in 2.x but I do not think it is a good idea in Java, at all.

Edit: to give you another example: Android Data binding throw ObservableField<T> can be easily wrapped in an Rx Observable<T> to receive every user modification. That field CAN be null and that's actually a perfectly valid value for it! Wrapping it in an Observable<Optional<T>> is an unnecessary complication. I expect you'll get a lot of hate for this decision from developers.

dimsuz commented 7 years ago

That field CAN be null and that's actually a perfectly valid value for it!

Than you can adapt it to more sane RxJava 2.x world by doing

observableFields.map(v -> if (v == null) { return NOT_FOUND } else { return v } )
danielesegato commented 7 years ago

@dimsuz what is observableFields in your example? On Rx 2.0 null can't be there and can't be mapped. also NOT_FOUND = ? If it's a string or an enum mapped to a Spinner (selector) where the user didn't picked anything / removed the pick?

Even worst what if it's an object of a class provided by a library? There are many cases like this. Just ignoring the fact that null is a perfectly valid value in Java is hiding your head in the sand.

Anything you do to make it work without null (in any situation) is a workaround that add unnecessary complexity.

dimsuz commented 7 years ago

Right, my example was kinda wrong.

I see null concept as a bit mixed thing, which can mean an error and an absence of value (which is itself can be seen as value). So it implicitly encodes these meanings, it's semantics is unclear because of this.

It's far better to have some explicit NOT_FOUND value.

If this library decided to go this more explicit way you can easily follow it. Even if some library does not follow this and has null objects signalling an absence of value, then you can write a simple adapter around it.

You can extend enum with a special value, or wrap it in Optional or something like this.

In absence of sum types in Java this is a tradeoff you get for better type checking && safer runtime...

But I agree, this is the kind of question which can easily result in flame wars, I guess we can go on and on about pros & cons.

I would say that developers of this library should decide.

JakeWharton commented 7 years ago

It's not RxJava's choice, it's in the reactive streams spec.

On Thu, Oct 27, 2016, 11:48 AM Daniele Segato notifications@github.com wrote:

This is really bad.

Nulls values are really needed in Java. You are forcing to clutter the API of our apps with Observable<Optional> when we could just use Observable.

I also don't like at all the NOT_FOUND suggestion from @akarnokd https://github.com/akarnokd and I'm specially concerned of this when you need to wrap others APIs (where you have no control on).

I can understand the decision about nulls in 2.x but I do not think it is a good idea in Java, at all.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/ReactiveX/RxJava/issues/4644#issuecomment-256608912, or mute the thread https://github.com/notifications/unsubscribe-auth/AAEEEcUjPI64SVmrVdRGck3ZMb12BLK4ks5q4IGEgaJpZM4KKlHC .

danielesegato commented 7 years ago

@dimsuz I agree that you can work around it, creating dedicated "NULL_VALUE" constants, wrapping in Optional classes or whatever.

I need to wrap I library that can return NULL values?

1.x: No problem, I just use emit it's values and handle them in an Observable, peace of cake. 2.x: Ah, nope I need to create a wrapper class (Optionable<> or whatever)

Let's now say you need to map those values to something else, using another library that interoperate with it:

1.x: No problem: .map() etc and you are good to go, that other library already handle nulls cause it has been writted to work with it 2.x: Since Optionable is now in the way i need to extract the value, then re-wrap the new value if that can be null.

Code is bound to get messier.

And backward compatibility? That will be an HELL. Libraries that are still 1.x will not be usable by 2.x because they'll start crashing on nulls. Overall I'm very concerned with this decision.

JakeWharton commented 7 years ago

It was decided nearly two years ago in the reactive streams spec.

On Thu, Oct 27, 2016, 3:21 PM dimsuz notifications@github.com wrote:

Right, my example was kinda wrong.

I see null concept as a bit mixed thing, which can mean an error and an absence of value (which is itself can be seen as value). So it implicitly encodes these meanings, it's semantics is unclear because of this.

It's far better to have some explicit NOT_FOUND value.

If this library decided to go this more explicit way you can easily follow it. Even if some library does not follow this and has null objects signalling an absence of value, then you can write a simple adapter around it.

You can extend enum with a special value, or wrap it in Optional or something like this.

In absence of sum types in Java this is a tradeoff you get for better type checking && safer runtime...

But I agree, this is the kind of question which can easily result in flame wars, I guess we can go on and on about pros & cons.

I would say that developers of this library should decide.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/ReactiveX/RxJava/issues/4644#issuecomment-256655345, or mute the thread https://github.com/notifications/unsubscribe-auth/AAEEEfZ0u-Fgn1swblW5kGIwBlIxhyQ3ks5q4LNdgaJpZM4KKlHC .

ZakTaccardi commented 7 years ago

@JakeWharton do you agree with the ReactiveStreams spec? (maybe there's a certain reason nulls are not allowed that I'm not aware of)

Maybe we can petition those maintaining the reactive streams spec to allow nulls?

danielesegato commented 7 years ago

@JakeWharton this one?

Do you know if I can find a discussion about this of 2 years ago? I'd like to understand the reasoning behind and what led to the decision. Thank you.

I've find this discussion and this one. I'm gonna need sometime to read them all :) can't now.

smaldini commented 7 years ago

the reason is that you can't buffer/queue nulls when doing message passing, and the spec is all about message passing.

vietj commented 7 years ago

@smaldini out of curiosity, how do you handle Mono<Void> in RSC without null ?

akarnokd commented 7 years ago

Rsc doesn't have a 0-1 type. Reactor's Mono<Void> is a Publisher and follows the same rules - it never emits onNext.

vietj commented 7 years ago

@akarnokd thanks for clarification

smaldini commented 7 years ago

Publisher<Void> is a valid Publisher that will never emit onNext indeed. That's one of the main semantic differences we have vs Maybe/Single/Completable in Reactor to still implement Publisher at every stage. In that case it pretty much leverages the deferred nature of Publisher#subscribe to implement a form of backpressure : don't do anything until subscribed.

ScottPierce commented 7 years ago

In the process of upgrading some libraries from 1.x to 2.x, this has shown itself to be very painful.

i.e. my company uses a library where we can subscribe to all changes to persisted values, and it's perfectly valid for a value not to be persisted yet, and thus be null.

Wrapping values in Optional seems annoying while programming in Kotlin, and creating a NOT_FOUND value for the various types also seems annoying. For instance, it's not obvious what value you'd use for a Date object.

I understand that the decision for the spec is outside the scope of this project, but I do find the change where it's now enforced in 2.0 to be frustrating, and felt the need to add my voice to the mix.

ZakTaccardi commented 7 years ago

Yep. Use Optainal

On Sun, Dec 4, 2016, 1:45 AM Scott Pierce notifications@github.com wrote:

In the process of upgrading some libraries from 1.x to 2.x, this has shown itself to be very painful.

i.e. my company uses a library where we can subscribe to all changes to persisted values, and it's perfectly valid for a value not to be persisted yet, and thus be null.

Wrapping values in Optional seems annoying while programming in Kotlin, and creating a NOT_FOUND value for the various types also seems annoying. For instance, it's not obvious what value you'd use for a Date object.

I understand that the decision for the spec is outside the scope of this project, but I do find the change where it's now enforced in 2.0 to be frustrating, and felt the need to add my voice to the mix.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/ReactiveX/RxJava/issues/4644#issuecomment-264687762, or mute the thread https://github.com/notifications/unsubscribe-auth/AAcJOj3vlX8IHH-YQDBjb0hzA5VbpO5yks5rEmF3gaJpZM4KKlHC .

ScottPierce commented 7 years ago

I ultimately chose the Optional approach, as it seems like it'll be more consistent than the alternatives. It's too early to tell how my team feels about it. I'll give it a few weeks.

ivnsch commented 7 years ago

How to handle this in Android Kotlin...? There are no optionals, since Android doesn't support Java 8, for lists it's also not possible to return empty lists, so all I can do is to return Object(), which basically makes me change the type of the observable to Object, losing type safety everywhere...?

Maybe I can use an "either" type or similar?

ScottPierce commented 7 years ago

@i-schuetz It's a crappy solution, but it works: https://github.com/memoizr/retro-optional

ivnsch commented 7 years ago

@ScottPierce thanks! I was able to solve it with funKTionale but there are some transitive dependency problems and I don't need all that just for the optionals...

I wonder, someone mentioned the use of rx's Maybe for this, though it's not quite the same thing. It would at least not involve adding another dependency... thoughts?

ScottPierce commented 7 years ago

As long as you are willing to change the way that you deal with streams, Maybe could work. I personally think that makes streams even more frustrating than Optional, but feel free to give it a shot.

If you are using Kotlin, we found that by adding some extension functions / properties, Optional is a lot easier to swallow. Namely:

val <T> Optional<T>.value: T?
    get() = if (isPresent) get() else null
ivnsch commented 7 years ago

@ScottPierce yeah I tried Maybe but there were some awkward parts, like getOrElse, in particular isDefined ... (seems to need something like blockingGet() != null? Not sure...). All in all it has a different purpose in mind so I'm using retro's Optional now, all good! I also came to the idea of adding those extensions as they were present in funKTionale, also to convert normal/Kotlin "optionals" objects to optionals...

Thanks!

artem-zinnatullin commented 7 years ago

You can try Koptional, it's small, made with usability in mind and tries to fit Kotlin's design, also has extensions for RxJava: filterSome() and filterNone()

https://github.com/gojuno/koptional

ubaierbhat commented 7 years ago

Why not add an implementation of Optional<T> in RxJava2 (or RxAndroid) since null is illegal now.

JakeWharton commented 7 years ago

Use java.util.Option

On Thu, Jul 27, 2017, 3:40 PM ubaierbhat notifications@github.com wrote:

Why not add an implementation of Optional in RxJava2 (or RxAndroid) since null is illegal now.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/ReactiveX/RxJava/issues/4644#issuecomment-318365037, or mute the thread https://github.com/notifications/unsubscribe-auth/AAEEEZj5wE952NoSe_ksWXaxsDoA1aY4ks5sSJM3gaJpZM4KKlHC .

ZakTaccardi commented 7 years ago

@JakeWharton but it's not an option on Android - java.util.Optional is Java 8

Sent from my Google Pixel XL using FastHub

akarnokd commented 7 years ago

Exactly. Targeting Java 6, there is no standard Optional and we didn't want to introduce yet another one.

JakeWharton commented 7 years ago

Use Guava's Optional.

Sent from my email client on my phone.

On Thu, Jul 27, 2017, 10:03 PM David Karnok notifications@github.com wrote:

Exactly. Targeting Java 6, there is no standard Optional and we didn't want to introduce yet another one.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/ReactiveX/RxJava/issues/4644#issuecomment-318470765, or mute the thread https://github.com/notifications/unsubscribe-auth/AAEEEcA4x83gaSBlA_Q4dz3KbbgdHIUDks5sSOz5gaJpZM4KKlHC .

yuriykulikov commented 7 years ago

You can use Guava Optional or Vavr option. Both work well with Immutables.io. If you are not using them and application size is an issue, you can throw in a simple optional implementation. It will be like 100 lines.

justjanne commented 7 years ago

So, @JakeWharton , the official solution is to double every short-lived event object, and the resulting GC load? That’s not even close to a usable option, this is never going to handle thousands of messages per second on a mobile phone when every single one has to be wrapped twice, unwrapped twice, and the GC has to collect all those objects again.

Are there any other solutions being worked on?

JakeWharton commented 7 years ago

Send batches.

justjanne commented 7 years ago

@JakeWharton Not an option. So the answer is "never use RxJava2" then?

JakeWharton commented 7 years ago

Use null object pattern.

On Thu, Sep 21, 2017 at 2:59 PM Janne Koschinski notifications@github.com wrote:

@JakeWharton https://github.com/jakewharton Not an option. So the answer is "never use RxJava2" then?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/ReactiveX/RxJava/issues/4644#issuecomment-331250170, or mute the thread https://github.com/notifications/unsubscribe-auth/AAEEEXAHKYMyjZVnPP0Ds8PiMOruoUaXks5skrIEgaJpZM4KKlHC .

halfhp commented 6 years ago

Use java.util.Option

How is this different than using null as a sentinel (aside from being less efficient), which was a big reason for not supporting null in the first place?

JakeWharton commented 6 years ago

Because it's explicitly not null allowing the library to use null internally as its own sentinel in queues.

On Fri, Oct 13, 2017, 12:05 PM Nick Fellows notifications@github.com wrote:

Use java.util.Option

How is this different than using null as a sentinel (aside from being less efficient), which was a big reason for not supporting null https://github.com/reactive-streams/reactive-streams-jvm/issues/204#issuecomment-70711355 in the first place?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/ReactiveX/RxJava/issues/4644#issuecomment-336495931, or mute the thread https://github.com/notifications/unsubscribe-auth/AAEEEf5dP5xkrFuT0u6T4qbYcTGasiRUks5sr4pDgaJpZM4KKlHC .

halfhp commented 6 years ago

Hmmm, as far as I can follow in the thread re the change, I didnt notice anyone mention that as a goal. Also seems a little odd for them to poop on using null as a sentinel, so they could turn around and use null as a sentinel :p

JakeWharton commented 6 years ago

Heh, yep!

Another advantage I perceive although I'm not sure it was a goal is that you never have to worry about null checking values in almost all callbacks from the library. Since there's no real way to propagate the fact that a value might be null down the stream this eliminated a good chunk of needless defensive coding in common code for me.

On Fri, Oct 13, 2017 at 12:09 PM Nick Fellows notifications@github.com wrote:

Hmmm, as far as I can follow in the thread re the change, I didnt notice anyone mention that as a goal. Also seems a little ironic for them to poop on using null as a sentinel, so they could turn around and use null as a sentinel :p

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/ReactiveX/RxJava/issues/4644#issuecomment-336496987, or mute the thread https://github.com/notifications/unsubscribe-auth/AAEEEf756ueYkAuuyjNCC1hquZBWzUDFks5sr4szgaJpZM4KKlHC .

akarnokd commented 6 years ago

Disallowing null was the decision the Reactive Streams project made. Compliance requires that we honor this as well. Knowing that flow items coming through onNext can never be null allowed us to utilize null as a sentinel in internal queues for example. So we didn't disallow null values so that we can optimize internal logic and afaik the decision wasn't about such possibility either. As Jake mentioned, it turned out non-null has other benefits.

romanpa commented 6 years ago

Hi

While we were using rx1 we had a lot of Single.just(null) , Single<Void> and things like this that in rx2 we are actually had been forced to represent as Completable. But it's impossible to chain completables in lazy way like .map(), .flatMap() and etc. in other types. Compeltable.andThen expects 'eager' completable. We are missing here a lazy 'andThen()' implementation, something like this:

public abstract class Completable implements CompletableSource {
    public final <T extends CompletableSource> Completable andThen(Supplier<T> defer) {
        return concatWith(Completable.defer(defer::get));
    }
}

This function would greatly simplify a people's life (refactoring of rx1 to rx2). And will eliminate the need of messing around with 'Optional' and generally will 'clean' a code.

akarnokd commented 6 years ago

@romanpa The answer is: use Completable.defer if you need to defer the creation of the Completable that is to follow and use andThen as usual. You can even implement it as a CompletableTransformer and use static imports to avoid spelling out factory classes.

romanpa commented 6 years ago

I need a lazy way to chain completables. This is my actual request. Completable as it's now, introduce inconsistent behavior in rx You can write Single.just(1).flatMap(o -> Single.just(o*2)) when Single.just(o*2) will be lazily created. And you cannot writeCompletable.complete().andThen(() -> lazyCompletable)

Of course that I can put every where Completable.defer , but it blows up the code , make it ugly and hard to read + if you have a huge amount of code like we do, it's a lot of work .

What is cleaner Completable.complete().andThen(() -> lazyCompletable) or Completable.complete().andThen(Completable.defer(() -> lazyCompletable))

romanpa commented 6 years ago

We could live with this limitation in rx1, but with restrictions of rx2 it's become a problem

akarnokd commented 6 years ago

Use static imports to remove some boilerplate, use Kotlin or keep using Single with an arbitrary non Void type and value.

import static io.reactivex.Completable.defer;

Completable.complete().andThen(defer(() -> lazyCompletable))
romanpa commented 6 years ago

All things that you are proposing are actually some sort of hacks. Kotlin is out of scope and over-complicated solution to this problem. Kind of swatting flies with a sledgehammer. Single.just(true) and things like this are also pretty ugly hack. When there is no meaning in returned value one should use Completable , shouldn't he? static defer - it's also hack that adds unnecessary boilerplate code.

When you use a library you expect it to be consistent with itself.

The need to use a hack in itself demonstrates that there is a problem. I am not talking here about some theoretical things. We are very heavy users of RX with tons of code. The move from rx1 to rx2 it's a very resource demanding for us. And if we are already doing it, we would like to do it in a right way.

justjanne commented 6 years ago

And wrapping Objects in Optional more than doubled my GC load, and the alternative was replacing null checks everywhere with ugly comparisons for equality (which is also slower, because Java special-cases null-comparisons for better performance, but can't do the same with == SomeInterface.NULL). Disallowing nulls may make sense in languages with value objects, in Java it's more than suboptimal.