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

Implement LambdaConsumerIntrospection #5590

Merged
merged 10 commits into from
Sep 12, 2017
Next Next commit
Implement HasDefaultErrorConsumer
Followup from #5569, and allows you to introspect if the resulting observer has missing error consumption and subsequently supplies a default (throwing) one.
  • Loading branch information
ZacSweers committed Sep 6, 2017
commit da4e67f7949bdf481fff622c20f670e8bf5afb77
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,12 @@
import io.reactivex.exceptions.*;
import io.reactivex.functions.*;
import io.reactivex.internal.disposables.DisposableHelper;
import io.reactivex.internal.functions.Functions;
import io.reactivex.observers.HasDefaultErrorConsumer;
import io.reactivex.plugins.RxJavaPlugins;

public final class LambdaObserver<T> extends AtomicReference<Disposable> implements Observer<T>, Disposable {
public final class LambdaObserver<T> extends AtomicReference<Disposable>
implements Observer<T>, Disposable, HasDefaultErrorConsumer {

private static final long serialVersionUID = -7251123623727029452L;
final Consumer<? super T> onNext;
Expand Down Expand Up @@ -101,4 +104,9 @@ public void dispose() {
public boolean isDisposed() {
return get() == DisposableHelper.DISPOSED;
}

@Override
public boolean hasMissingErrorConsumer() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A test would be great.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

return onError == Functions.ON_ERROR_MISSING;
}
}
18 changes: 18 additions & 0 deletions src/main/java/io/reactivex/observers/HasDefaultErrorConsumer.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
package io.reactivex.observers;

import io.reactivex.annotations.Experimental;

/**
* An interface that indicates that the implementing type has default implementations for error consumption.
*/
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@since 2.1.4 - experimental

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Experimental
public interface HasDefaultErrorConsumer {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should find a better name for this interface for several reasons:

  1. HasDefaultErrorConsumer sounds like marker interface that you need to use with instanceof to get boolean result.
  2. Interface HasDefaultErrorConsumer and method name hasMissingErrorConsumer() are inconsistent.
  3. Extensibility — do we plan to extend it with other functions to check onNext/onComplete implementations?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd definitely get rid of "default" and "missing" from both. The negated method is really confusing. There's already Has* types which do similar things so I don't agree that it needs changed.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, last sentence dropped from my comment for some reason.

Depending on answer to №3 I have following interface names on my mind ObserverIntrospection/SubscriberIntrospection or ErrorConsumerIntrospection, hope someone has better suggestions :)

The negated method is really confusing.

Huh, indeed! Didn't notice that until you pointed

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I let you people work out the naming.

Copy link
Contributor Author

@ZacSweers ZacSweers Sep 8, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for the method, how about just a simple isNotImplemented() or isThrowing()?

Assuming the interface itself gets named HasErrorConsumer?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

People will most likely mistreat CompositeObserver and think of it as of a combination of observers because that's how CompositeDisposable and CompositeException work.

ObserverInstrumentation/ObserverIntrospection? Such a name also more clearly delivers the intention of consume-only public API

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Composite is misleading and will show up in content assist when the developer types Composite

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

to get to the disposable/exception variant mentioned.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd call it LambdaConsumerIntrospection and hasCustomOnError()

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good points. The Introspection moniker feels off to me, mostly because I consider introspection an act and don't necessarily think of it as a noun/type at first. I don't know what a better word would be though. Updated in bae194c


/**
* @return {@code true} if the implementation is missing an error consumer and thus using a throwing default
* implementation. Returns {@code false} if a concrete error consumer implementation was supplied.
*/
@Experimental
boolean hasMissingErrorConsumer();

}