Uh oh!
There was an error while loading. Please reload this page.
adding {Flowable, Observable}::justOnce operator - #499
Conversation
| template <typename T> | ||
| static Reference<Observable<T>> justOnce(T value) { | ||
| auto lambda = [value = std::move(value), used = false](Reference<Observer<T>> observer) mutable { | ||
| CHECK(!used) << "justOnce value was already used"; |
There was a problem hiding this comment.
I think this should resolve with an exception in the other case, crashing doesn't seem correct.
There was a problem hiding this comment.
I was going back and forth about this. On one hand I agree with you that crashing is not good. On the other hand this is violation of the API and the check is just checking that you are using it correctly. Debugging in the case of CHECK is much easier to debug.
I can also add DCHECK and exception instead of CHECK.
Anybody else want's to weigh in?
There was a problem hiding this comment.
I think we shouldn't crash here. If someone tries to resubscribe to a JustOnce operator, then they should probably get sent an onError.
| auto value = std::make_unique<int>(123456); | ||
| auto flowable = Flowables::justOnce(std::move(value)); | ||
| EXPECT_EQ(std::size_t{1}, Refcounted::objects()); |
| TEST(Observable, SingleMovable) { | ||
| auto value = std::make_unique<int>(123456); | ||
| auto observable = Observables::justOnce(std::move(value)); |
There was a problem hiding this comment.
nit: one more case to check if it fires the exception correctly
There was a problem hiding this comment.
Will cover in the next diff. Thank you for your feedback!
this is a version of the just operator which works with movable types. The aim is to enable using just operator with the restriction that you can subscribe to the flowable/observable only once.