Skip to content

Instantly share code, notes, and snippets.

@mr-archano
Last active August 29, 2015 14:10
Show Gist options
  • Star 0 You must be signed in to star a gist
  • Fork 0 You must be signed in to fork a gist
  • Save mr-archano/0d1c5885229229bed637 to your computer and use it in GitHub Desktop.
Save mr-archano/0d1c5885229229bed637 to your computer and use it in GitHub Desktop.
I didn't want to pollute the discussion started in RxAndroid/#12 (https://github.com/ReactiveX/RxAndroid/issues/12), so I drafted a solution that collects some of the neat ideas showcased in there.
// NOTE: `OperatorSubscribeUntil` class is borrowed from @dlew's PR (https://github.com/dlew/RxAndroid/blob/dlew/lifecycle-observable/rxandroid/src/main/java/rx/android/lifecycle/OperatorSubscribeUntil.java)
package rx.android;
import rx.Observable;
import rx.Subscriber;
import rx.observers.SerializedSubscriber;
/**
* Returns an Observable that emits the items from the source Observable until another Observable
* emits an item.
* <p>
* Unlike takeUntil, this choose to unsubscribe the parent rather than calling onComplete().
*/
final class OperatorSubscribeUntil<T, R> implements Observable.Operator<T, T> {
private final Observable<? extends R> other;
public OperatorSubscribeUntil(final Observable<? extends R> other) {
this.other = other;
}
@Override
public Subscriber<? super T> call(final Subscriber<? super T> child) {
final Subscriber<T> parent = new SerializedSubscriber<T>(child);
other.unsafeSubscribe(new Subscriber<R>(child) {
@Override
public void onCompleted() {
parent.unsubscribe();
}
@Override
public void onError(Throwable e) {
parent.onError(e);
}
@Override
public void onNext(R t) {
parent.unsubscribe();
}
});
return parent;
}
}
package rx.android;
import rx.Observable;
import rx.subjects.PublishSubject;
import java.util.HashMap;
import java.util.Map;
public class EventDrivenUnsubscriber<E extends EventDrivenUnsubscriber.Event> {
private final Map<E, PublishSubject<E>> subjects;
public EventDrivenUnsubscriber() {
this.subjects = new HashMap<E, PublishSubject<E>>();
}
public <T> Operator<T, T> bindTo(E event) {
PublishSubject<E> subject = subjects.get(event);
if (subject == null) {
subject = PublishSubject.create();
subjects.put(event, subject);
}
return new OperatorSubscribeUntil<T, E>(subject);
}
public void unsubscribeAll(E event) {
PublishSubject<E> subject = subjects.get(event);
if (subject != null) {
subject.onNext(event);
}
}
static class Event {
private final String name;
protected Event(String name) {
this.name = name;
}
@Override
public boolean equals(Object o) {
if (this == o) return true;
if (!(o instanceof Event)) return false;
Event event = (Event) o;
return name.equals(event.name);
}
@Override
public int hashCode() {
return name.hashCode();
}
}
}
package rx.android;
public class ActivityLifeCycleEvent extends EventDrivenUnsubscriber.Event {
public static final ActivityLifeCycleEvent ON_PAUSE = new ActivityLifeCycleEvent("onPause()");
public static final ActivityLifeCycleEvent ON_RESUME = new ActivityLifeCycleEvent("onResume()");
// TODO add other relevant events here...
private ActivityLifeCycleEvent(String name) {
super(name);
}
}
package rx.android.samples;
import android.app.Activity;
import android.os.Bundle;
import rx.Observable;
import rx.android.ActivityLifeCycleEvent;
import rx.android.EventDrivenUnsubscriber;
public class FooActivity extends Activity {
private final EventDrivenUnsubscriber<ActivityLifeCycleEvent> unsubscriber = new EventDrivenUnsubscriber<ActivityLifeCycleEvent>();
@Override
protected void onCreate(Bundle savedInstanceState) {
super.onCreate(savedInstanceState);
Observable.just("wut wut")
.lift(unsubscriber.bindTo(ActivityLifeCycleEvent.ON_PAUSE))
...;
}
@Override
protected void onPause() {
super.onPause();
unsubscriber.unsubscribeAll(ActivityLifeCycleEvent.ON_PAUSE);
}
}
@mr-archano
Copy link
Author

Reporting @dlew comment from the original thread:

From your gist it looks like your solution still requires inheritance. More
of it than mine does.

That solution looks a lot like my original work. The problem is that it
adds a lot of unnecessary framework. It stores state outside of a reactive
context. It depends on smart overrides of methods in the life cycle.

When I really reduced the problem to its core parts I realized the
Observable of the life cycle was all I really needed.

@mr-archano
Copy link
Author

Comments inlined:

From your gist it looks like your solution still requires inheritance. More
of it than mine does.

Inheritance in this solution is used uniquely to build the life cycle events ontology. This has been done on purpose to be able to make a distinction between a fragment and activity lifecycle rather than use the same superset of events for both - that I think is wrong.
Also the type binding will force the user to use EventDrivenUnsubscriber along with ActivityLifeCycleEvent or FragmentLifeCycleEvent avoiding checks in the binding process (ie: binding unsubscribing to CREATE_VIEW in an Activity) because the compiler will check that for you.

The problem is that it adds a lot of unnecessary framework. It stores state outside of a reactive
context.

Sorry, but I don't see how the above is true. Storing a PublishSubject is storing state? It's just needed to trigger the unsubscribing. Also the map can be replaced by a single PublishSubject and a filter can be used to make only the relevant event to pass through.

It depends on smart overrides of methods in the life cycle.

You cannot avoid that in any way. At some point you need to override the life cycle callbacks in your fragment/activity or - worse - provide a pre-cooked solution (eg: LifecycleTrackedActivity/LifecycleTrackedFragment).

When I really reduced the problem to its core parts I realized the
Observable of the life cycle was all I really needed.

This is arguable, but in general I don't really like to be forced to use 2 pieces to build my solution while I can just use a simpler one.

@dlew
Copy link

dlew commented Nov 24, 2014

Inheritance in this solution is used uniquely to build the life cycle events ontology.

Here I was mostly objecting to you claiming that your solution uses composition over inheritance, where there is clearly inheritance baked into your solution. In my solution there is no inheritance necessary, in fact (in the library; naturally you must extend Activity or Fragment in your app, but you do that anyways for normal app development). Which one is relying more on inheritance?

If your biggest objection is having a single enum for both Activity and Fragment events, then there's a simple solution: two enums instead of one. Then there can be no mixing of the two, but it introduces a bit of code duplication (but just as an implementation detail).

Storing a PublishSubject is storing state?

Your EventDrivenUnsubscriber is keeping track of all the lifecycle events that have subscribers which need to be unsubscribed. I'd call that storing state.

If you switch to just having a single PublishSubject that filters all the lifecycle events... then you've come up with my solution again, only you've wrapped it in EventDrivenUnsubscriber.

Compare either of those with Observable<LifecycleEvent>, which doesn't give a crap about who is using it. It just exists to be consumed.

It depends on smart overrides of methods in the life cycle.

You cannot avoid that in any way.

But you can avoid making it harder. With just an Observable then it's the common onNext() paradigm that RxJava users are used to. With EventDrivenUnsubscriber you're forced to learn a second paradigm.

This is arguable, but in general I don't really like to be forced to use 2 pieces to build my solution while I can just use a simpler one.

I think this is at the root of our disagreement. I believe in encapsulation and separation of concerns where possible. I think this is one of those cases, and I consider it simpler than the two pieces combined.

@mr-archano
Copy link
Author

LoL, the discussion is going pretty serious here, I like it 😄

What I definitely wanted to "claim" is that the final user doesn't need to extend any RxFragment/RxAndroid to make it work, it just needs to delegate to an EventDrivenUnsubscriber<ActivityLifeCycleEvent> (as opposed to other examples in the main discussion showing abstract fragments/activities).

Also when I'm referring to 2 pieces this is exactly what I mean: a user needs to extend from RxFragment and use the LifecycleObservable. Because I reckon that in order to define an Observable<LifecycleEvent> you will eventually need to propose to extend a fragment/activity as part of your solution, because you want to minimise/hide the boilerplate code needed to track the component lifecycle and emit events accordingly. You can avoid it but then you end up with a lot of duplication. Or maybe you can wrap the boilerplate in a helper class, but you will still have 2 pieces.

So basically if a user doesn't want to extend any fragment at all there is no much choice I would say. And now maybe you can get better my point of view.

I'm not here to discuss how to apply SOLID to anyone's code, I perfectly understand how you ended up with an Observable<LifecycleEvent> and I agree that looks pretty neat. I'm not here to claim anything, just to discuss a solution that will be bundled in a library used by people with heterogeneous needs. I'm not even a maintainer, I just enjoy interesting discussions.

@dlew
Copy link

dlew commented Nov 25, 2014

Delegating to EventDrivenUnsubscriber<LifecycleEvent> is exactly the same as delegating to Observable<LifecycleEvent>, since in both cases you need to write all the lifecycle handling code yourself... it's just that one is a base class of RxJava and the other is a wrapper that does the same thing.

Your solution still requires all the same extensions of fragment/activity to reduce boilerplate. Unless you're proposing that EventDrivenUnsubscriber requires people to write their own event system, at which point it's basically no better than not having any framework for this at all (e.g., why not just use a CompositeSubscription?).

@Alexander--
Copy link

If your biggest objection is having a single enum for both Activity and Fragment events, then there's a simple solution: two enums instead of one.

Or even better - just create ad-hoc enumeration via custom resource type annotations. Java enums are great, but this method avoids their overhead of using them when their class-based nature is redundant, and allows arbitrary number of new items to be appended from different places.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment