-
Notifications
You must be signed in to change notification settings - Fork 7.6k
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
Add Observable.fromCallable() as a companion for Observable.defer() #3154
Conversation
Also, I see that for Rx newcomers (some of my coworkers, for example) is not very clear why they need to return |
Sounds ok to me. A name using the |
We can call it |
We can't call it from because of potential conflict in dynamic languages. I'd prefer start or startAsync . |
@@ -1069,6 +1069,10 @@ public void call(Subscriber<? super R> o) { | |||
return create(new OnSubscribeDefer<T>(observableFactory)); | |||
} | |||
|
|||
public static <T> Observable<T> lazy(Func0<T> lazyFunc) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Func0<? extends T>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed
If we can not use |
Whatever the name is ( |
Operator is ready for next review!
Another naming suggestion: |
Not sure about this, because I don't know what variant will be better (more efficient, maintainable, etc) |
There is special logic in |
You can't return the scalar observable because the value has to be ready at assembly time and be constant. Any side effect would only happen once as well. |
@@ -1069,6 +1069,11 @@ public void call(Subscriber<? super R> o) { | |||
return create(new OnSubscribeDefer<T>(observableFactory)); | |||
} | |||
|
|||
@Experimental | |||
public static <T> Observable<T> lazy(Func0<? extends T> lazyFunc) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd consider Callable to allow IO operations without the need to do try-catch. An overload won't work due to dynamic languages.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I've thought about this too, I'll take a look what I can do after work.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done!
@akarnokd I'll try to send in an example of what I mean but I don't see any reason why |
Instead of modifying ScalarSynchronousObservable, you could add a new case to the type check in merge. |
check out #3163 (made it PR so it can be discussed over there). |
There are several problems with your proposal
|
👍 |
Switched to I hope So, I guess it's time to select the name for this operator and add javadoc. |
I would also like to see the observables produced by |
bc77544
to
92319a2
Compare
Added javadoc and squashed. P.S. Today I used |
All Observables are lazy. Why can't you just use Observable.create()? |
@staltz That would conflict with My new name proposals:
|
|
It's not true, in Java (and all JVM langs that I know) argument of a method computed before actual invocation of this method. For example if you have method So, in my opinion
|
Sorry to be late into the game here, but it seems you really want an overload of I am surprised this is a common scenario since it feel super narrow. Personally I think the longer Observable
.defer(() -> {
Value value0, value1, ...valuen-1 = ...;
// some code
return Observable.just(value0, ..., valuen-1);
})
.subscribeOn(...) Anyway, just my 2 cents. |
7d4f825
to
d9c3482
Compare
Renamed operator from
Surprise surprise — nobody likes boilerplate. Also, if you need to defer function that throws checked exception via There is even post from @dlew Deferring Observable code until subscription in RxJava which, I guess, makes this problem more visible. |
You have just solved the boilerplate for one very particular case that does not generalize to the other overloads of |
I'm not a fan. I prefer composability. You can easily compose existing operators to recreate |
I called this operator |
d9c3482
to
0c8b250
Compare
Nice and understandable name 👍 Renamed to |
@akarnokd @benjchristensen is there anything that blocks this PR? |
I'm totally in favor of this addition so we can start promoting it instead of |
Yes! I see some confusion when newbies hear: "To defer some code and turn it into I literally can read "wtf"s in their eyes when they ask — "So, I need to create |
👎 this is a less general use case than an |
We can always optimize later. |
I see the compelling usability need (easy to type, simple to find) so I see that users would adopt it but it would detract from the optimized path. I don't think this will perform well if its adopted in the Netflix ecosystem. Many other users here have raised objections as this use case can be handled with existing operators and that composition would skip the merge ring buffer allocation/draining. |
That's not how the merge optimizations work. |
Oh, you're right. Merge can't optimize either path. At least not as a scalar. It still has to subscribe to de defer. My mistake. |
My objections were on faulty assumptions. I take back my -1. 👍 |
Great! |
Add Observable.fromCallable() as a companion for Observable.defer()
Great addition! Thanks 👍 |
That is generally why On Sun, Sep 6, 2015 at 2:13 AM, Jens Driller notifications@github.com
|
Yep, this is a new operator.
Motivation?
Observable.defer()
requires function that returnsObservable<T>
when usually we don't want to createObservable<T>
manually, we just want to defer execution of some function.Like this:
Instead of this:
And more important case with deferring code that throws checked exceptions:
Instead of this:
I'd use name
defer
but both methods will have same type erasure (Func0<Observable>
andFunc0<T>
), so I had to use a different name.Questions:
If the decision about this operator will be positive — I'll add javadoc to this PR and then create separate PR for
Single.fromCallable()
.