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

Android : how to manage screen rotation with observer #386

Closed
pommedeterresautee opened this Issue Sep 17, 2013 · 22 comments

Comments

Projects
None yet
5 participants
@pommedeterresautee

Hi,

Since I have discovered Rx java I have replaced all my async loaders by observers and observables.

That's very cool to easily get an error to the UI thread.

I have still a question.

As you know when the screen rotate the activity is killed and recreated.

I am wondering how should I do to resubscribe to the same observable launched before the rotation.

Is there a pattern I have not think of? Or is it a feature to come?

I am very new to reactive programming and I perfectly realize I am using 1% of the power of Rx java but can you help me to reach the 2%?

Thank you

@pommedeterresautee

This comment has been minimized.

Show comment
Hide comment
@pommedeterresautee

pommedeterresautee Sep 17, 2013

Something else, but related... is there a way to stop an observable during its work? I noticed that when onError is called the process is stopped, is there a way to call the stop method from the main thread?

Something else, but related... is there a way to stop an observable during its work? I noticed that when onError is called the process is stopped, is there a way to call the stop method from the main thread?

@benjchristensen

This comment has been minimized.

Show comment
Hide comment
@benjchristensen

benjchristensen Sep 19, 2013

Member

I don't have experience with Android so can't help on this. Perhaps the RxJava Google Group would be a better place to ask?

https://groups.google.com/forum/#!forum/rxjava

Member

benjchristensen commented Sep 19, 2013

I don't have experience with Android so can't help on this. Perhaps the RxJava Google Group would be a better place to ask?

https://groups.google.com/forum/#!forum/rxjava

@pommedeterresautee

This comment has been minimized.

Show comment
Hide comment
@pommedeterresautee

pommedeterresautee Sep 23, 2013

Hi @benjchristensen ,

Indeed it s not really an Android issue. It s a more general issue which may happen more often on Android.

The point is that I am getting many

java.lang.RuntimeException: Error occurred when trying to propagate error to Observer.onError

The reason is easy to understand, the activity which is also the subscriber has been closed (because the screen rotate, but we don't really care) before the observable catch an error and try to send it back to the subscriber.

Because of the way SafeObserver works... it crash all the application.

 @Override
    public void onError(Throwable e) {
        if (isFinished.compareAndSet(false, true)) {
            try {
                actual.onError(e);
            } catch (Throwable e2) {
                if (e2 instanceof OnErrorNotImplementedException) {
                    /**
* onError isn't implemented so throw
*
* https://github.com/Netflix/RxJava/issues/198
*
* Rx Design Guidelines 5.2
*
* "when calling the Subscribe method that only has an onNext argument, the OnError behavior will be
* to rethrow the exception on the thread that the message comes out from the observable sequence.
* The OnCompleted behavior in this case is to do nothing."
*/
                    throw (OnErrorNotImplementedException) e2;
                } else {
                    // if the onError itself fails then pass to the plugin
                    // see https://github.com/Netflix/RxJava/issues/216 for further discussion
                    RxJavaPlugins.getInstance().getErrorHandler().handleError(e);
                    RxJavaPlugins.getInstance().getErrorHandler().handleError(e2);
                    // and throw exception despite that not being proper for Rx
                    // https://github.com/Netflix/RxJava/issues/198
                    throw new RuntimeException("Error occurred when trying to propagate error to Observer.onError", new CompositeException(Arrays.asList(e, e2)));
                }
            }
            // auto-unsubscribe
            subscription.unsubscribe();
        }
    }

So in a general, is there a way to deal with an error reported AFTER the subscriber unsubscribe and even disappears?

Hi @benjchristensen ,

Indeed it s not really an Android issue. It s a more general issue which may happen more often on Android.

The point is that I am getting many

java.lang.RuntimeException: Error occurred when trying to propagate error to Observer.onError

The reason is easy to understand, the activity which is also the subscriber has been closed (because the screen rotate, but we don't really care) before the observable catch an error and try to send it back to the subscriber.

Because of the way SafeObserver works... it crash all the application.

 @Override
    public void onError(Throwable e) {
        if (isFinished.compareAndSet(false, true)) {
            try {
                actual.onError(e);
            } catch (Throwable e2) {
                if (e2 instanceof OnErrorNotImplementedException) {
                    /**
* onError isn't implemented so throw
*
* https://github.com/Netflix/RxJava/issues/198
*
* Rx Design Guidelines 5.2
*
* "when calling the Subscribe method that only has an onNext argument, the OnError behavior will be
* to rethrow the exception on the thread that the message comes out from the observable sequence.
* The OnCompleted behavior in this case is to do nothing."
*/
                    throw (OnErrorNotImplementedException) e2;
                } else {
                    // if the onError itself fails then pass to the plugin
                    // see https://github.com/Netflix/RxJava/issues/216 for further discussion
                    RxJavaPlugins.getInstance().getErrorHandler().handleError(e);
                    RxJavaPlugins.getInstance().getErrorHandler().handleError(e2);
                    // and throw exception despite that not being proper for Rx
                    // https://github.com/Netflix/RxJava/issues/198
                    throw new RuntimeException("Error occurred when trying to propagate error to Observer.onError", new CompositeException(Arrays.asList(e, e2)));
                }
            }
            // auto-unsubscribe
            subscription.unsubscribe();
        }
    }

So in a general, is there a way to deal with an error reported AFTER the subscriber unsubscribe and even disappears?

@benjchristensen

This comment has been minimized.

Show comment
Hide comment
@benjchristensen

benjchristensen Sep 27, 2013

Member

So in a general, is there a way to deal with an error reported AFTER the subscriber unsubscribe and even disappears?

The event is not relevant to the subscription at that point, since the subscriber is gone.

Because of the way SafeObserver works... it crash all the application.

What about SafeObserver would crash the application?

java.lang.RuntimeException: Error occurred when trying to propagate error to Observer.onError

That error does not mean the Observer is unsubscribed, that means the Observer is failing to handle the onError event and thus there is no choice but to throw.

So why is the Observer.onError failing when an exception is passed to it?

I think this happens because we can not disable HandlerThreadScheduler when the activity stops.

Is the activity you're referring to an Observable or an Observer?

If it is an Observable then it should emit an onCompleted and be done.

If is is an Observer then it should unsubscribe from the Subscription it received when it called Observable.subscribe and it will give the Observable a chance to shutdown and clean up.

The Scheduler does not have a cancel on it because what would that mean? Scheduling is per task (the schedule methods), and when something is scheduled it returns a Subscription. Thus, the Subscription is cancelled (Subscription.unsubscribe), not the entire Scheduler.

If you can post a unit test demonstrating the issue that would help.

Member

benjchristensen commented Sep 27, 2013

So in a general, is there a way to deal with an error reported AFTER the subscriber unsubscribe and even disappears?

The event is not relevant to the subscription at that point, since the subscriber is gone.

Because of the way SafeObserver works... it crash all the application.

What about SafeObserver would crash the application?

java.lang.RuntimeException: Error occurred when trying to propagate error to Observer.onError

That error does not mean the Observer is unsubscribed, that means the Observer is failing to handle the onError event and thus there is no choice but to throw.

So why is the Observer.onError failing when an exception is passed to it?

I think this happens because we can not disable HandlerThreadScheduler when the activity stops.

Is the activity you're referring to an Observable or an Observer?

If it is an Observable then it should emit an onCompleted and be done.

If is is an Observer then it should unsubscribe from the Subscription it received when it called Observable.subscribe and it will give the Observable a chance to shutdown and clean up.

The Scheduler does not have a cancel on it because what would that mean? Scheduling is per task (the schedule methods), and when something is scheduled it returns a Subscription. Thus, the Subscription is cancelled (Subscription.unsubscribe), not the entire Scheduler.

If you can post a unit test demonstrating the issue that would help.

@zsxwing

This comment has been minimized.

Show comment
Hide comment
@zsxwing

zsxwing Sep 27, 2013

Member

Sorry that I misunderstood this issue. I deleted my comments.

Member

zsxwing commented Sep 27, 2013

Sorry that I misunderstood this issue. I deleted my comments.

@pommedeterresautee

This comment has been minimized.

Show comment
Hide comment
@pommedeterresautee

pommedeterresautee Sep 29, 2013

@benjchristensen Thank you for your answer (-> questions :-) )

It made me search in the right direction.

I think I know understand what was happening.

On Android, UI stuff is realized in an Activity.

If a user rotates the phone (say from Portrait to Landscape) the Activity is killed. A new one is then recreated.

Of course, in the rotation you loose everything. There are some techniques to keep variable values however. But in general it has to be stored in a bundle object.... Android boring stuff.

Anyway, in my first implementation I followed the blog post from @mttkay without really understanding how Rx works inside. So the Activity class was implementing the Observer interface.

It means that if the user rotate its phone, you better have to call unsubscribe method or you will use a zombie Activity / Observer.

Unfortunately I look at the source code of Rx and I understood that Observable built outside Rx are wrapped inside a SafeObservable, and that SafeObservable will keep a hard reference to the Observer preventing it to be nullified + garbage collected.

In normal case, it should not be an issue because unsubscribe should have been called, right? but unfortunately in some cases I have still not found why, unsubscribe method is not called (lots of corner case in my app).

At the end of the story I get a call to onError() because an exception has been released in the working thread inside the Observable, this method is the one from an Observer which is also an Activity, and this Observer/Activity should have been destroyed -> doing stuff in a dead but not destroyed activity = crash the application.

Based on the comment from @mttkay I have written a simple Proxy Observer to avoid this situation (miss the unsubscribe + Observer has disappeared).

import java.lang.ref.WeakReference;

import rx.Observer;

public class AndroidProxyObserver {

    public static <T> Observer<T> secure(Observer<T> obs){
        return new ProxyObserver<T>(obs);
    }

    private static class ProxyObserver<T> implements Observer<T>{
        final private WeakReference<Observer<T>> mCallBack;
        private ProxyObserver(Observer<T> obs){
            mCallBack = new WeakReference<Observer<T>>(obs);
        }

        @Override
        public void onCompleted() {
            if(mCallBack.get() != null) {
                mCallBack.get().onCompleted();
            }
        }

        @Override
        public void onError(Throwable throwable) {
            if(mCallBack.get() != null) {
                mCallBack.get().onError(throwable);
            }
        }

        @Override
        public void onNext(T object) {
            if(mCallBack.get() != null) {
                mCallBack.get().onNext(object);
            }
        }
    }
}

Nothing complex, just a weak reference, so if the Activity is destroyed, there is no more hard reference to it to avoid cleaning / garbage collection. The Observer /Activity will be killed at the device rotation and no more call to onError() method is then possible because the WeakReference wrapper will be empty.

May be this code should be added to the Android contrib folder, I will wait comments from you or @mttkay or whoever would see anything interesting in this simple snippet, may be I am missing something else!

Moreover, I am trying to build a class to manage easily the device rotation (or any Activity/Observer destruction) in this Gist : https://gist.github.com/pommedeterresautee/6752846

I am waiting opinion from advanced RxJava user to see if it s useful / correct / has to be committed.

Again, thank you for having help to build such tool, I am very happy when using it!

Regards

@benjchristensen Thank you for your answer (-> questions :-) )

It made me search in the right direction.

I think I know understand what was happening.

On Android, UI stuff is realized in an Activity.

If a user rotates the phone (say from Portrait to Landscape) the Activity is killed. A new one is then recreated.

Of course, in the rotation you loose everything. There are some techniques to keep variable values however. But in general it has to be stored in a bundle object.... Android boring stuff.

Anyway, in my first implementation I followed the blog post from @mttkay without really understanding how Rx works inside. So the Activity class was implementing the Observer interface.

It means that if the user rotate its phone, you better have to call unsubscribe method or you will use a zombie Activity / Observer.

Unfortunately I look at the source code of Rx and I understood that Observable built outside Rx are wrapped inside a SafeObservable, and that SafeObservable will keep a hard reference to the Observer preventing it to be nullified + garbage collected.

In normal case, it should not be an issue because unsubscribe should have been called, right? but unfortunately in some cases I have still not found why, unsubscribe method is not called (lots of corner case in my app).

At the end of the story I get a call to onError() because an exception has been released in the working thread inside the Observable, this method is the one from an Observer which is also an Activity, and this Observer/Activity should have been destroyed -> doing stuff in a dead but not destroyed activity = crash the application.

Based on the comment from @mttkay I have written a simple Proxy Observer to avoid this situation (miss the unsubscribe + Observer has disappeared).

import java.lang.ref.WeakReference;

import rx.Observer;

public class AndroidProxyObserver {

    public static <T> Observer<T> secure(Observer<T> obs){
        return new ProxyObserver<T>(obs);
    }

    private static class ProxyObserver<T> implements Observer<T>{
        final private WeakReference<Observer<T>> mCallBack;
        private ProxyObserver(Observer<T> obs){
            mCallBack = new WeakReference<Observer<T>>(obs);
        }

        @Override
        public void onCompleted() {
            if(mCallBack.get() != null) {
                mCallBack.get().onCompleted();
            }
        }

        @Override
        public void onError(Throwable throwable) {
            if(mCallBack.get() != null) {
                mCallBack.get().onError(throwable);
            }
        }

        @Override
        public void onNext(T object) {
            if(mCallBack.get() != null) {
                mCallBack.get().onNext(object);
            }
        }
    }
}

Nothing complex, just a weak reference, so if the Activity is destroyed, there is no more hard reference to it to avoid cleaning / garbage collection. The Observer /Activity will be killed at the device rotation and no more call to onError() method is then possible because the WeakReference wrapper will be empty.

May be this code should be added to the Android contrib folder, I will wait comments from you or @mttkay or whoever would see anything interesting in this simple snippet, may be I am missing something else!

Moreover, I am trying to build a class to manage easily the device rotation (or any Activity/Observer destruction) in this Gist : https://gist.github.com/pommedeterresautee/6752846

I am waiting opinion from advanced RxJava user to see if it s useful / correct / has to be committed.

Again, thank you for having help to build such tool, I am very happy when using it!

Regards

@zsxwing

This comment has been minimized.

Show comment
Hide comment
@zsxwing

zsxwing Sep 30, 2013

Member

I think you need to call mSubscription.unsubscribe before the Activity is destroyed (e.g. call in Activity.onStop or Activity.onDestroy). After that, the new events in the Scheduler will be dropped.

Member

zsxwing commented Sep 30, 2013

I think you need to call mSubscription.unsubscribe before the Activity is destroyed (e.g. call in Activity.onStop or Activity.onDestroy). After that, the new events in the Scheduler will be dropped.

@mttkay

This comment has been minimized.

Show comment
Hide comment
@mttkay

mttkay Sep 30, 2013

Contributor

@pommedeterresautee

We use a similar approach i.e. using a WeakReference. However, the class we use does some additional things to make it more sound when being used to update the UI. We use an observer implementation which binds a fragment to a weak reference. In every callback it first checks if

a) the reference is still set (that's what you're doing too)
b) if the fragment is still attached (that's what you're not doing)

if you don't do b), your app might still crash since getActivity returns null when the fragment is not attached. If you don't use fragments (I hope you do!) then you should probably check for isFinishing and drop the message if that's true.

That class I wanted to contribute back, since it is production proven for a few months now and has proper unit tests and everything.

However, I actually like the idea of decoupling the two concerns. i.e. I see it could be beneficial (although slightly more complex to use) to have a WeakObserver (I think AndroidProxyObserver is probably not a good name since it doesn't convey any meaning) which could wrap a FragmentObserver or ActivityObserver which then do the above mentioned extra checks necessary when one wants to update the UI in onCompleted.

@benjchristensen Do you think a WeakObserver is something that should be part of the core module? It sounds more universally applicable than to just Android. It's simply an observer which doesn't guarantee to deliver any of the messages. (That might or might not be in violation with the design guidelines, but we actually do require this behaviour on Android to not leak resources.)

Contributor

mttkay commented Sep 30, 2013

@pommedeterresautee

We use a similar approach i.e. using a WeakReference. However, the class we use does some additional things to make it more sound when being used to update the UI. We use an observer implementation which binds a fragment to a weak reference. In every callback it first checks if

a) the reference is still set (that's what you're doing too)
b) if the fragment is still attached (that's what you're not doing)

if you don't do b), your app might still crash since getActivity returns null when the fragment is not attached. If you don't use fragments (I hope you do!) then you should probably check for isFinishing and drop the message if that's true.

That class I wanted to contribute back, since it is production proven for a few months now and has proper unit tests and everything.

However, I actually like the idea of decoupling the two concerns. i.e. I see it could be beneficial (although slightly more complex to use) to have a WeakObserver (I think AndroidProxyObserver is probably not a good name since it doesn't convey any meaning) which could wrap a FragmentObserver or ActivityObserver which then do the above mentioned extra checks necessary when one wants to update the UI in onCompleted.

@benjchristensen Do you think a WeakObserver is something that should be part of the core module? It sounds more universally applicable than to just Android. It's simply an observer which doesn't guarantee to deliver any of the messages. (That might or might not be in violation with the design guidelines, but we actually do require this behaviour on Android to not leak resources.)

@pommedeterresautee

This comment has been minimized.

Show comment
Hide comment
@pommedeterresautee

pommedeterresautee Sep 30, 2013

@zsxwing of course I have such call in onDestroy method.

However, I can't reproduce exactly what happens when the application crashes but in my observable (the one having issues) I am executing some shell commands , and in some cases, for some reasons, unsubscribe doesn't kill immediately the working thread. Most of the time it does but not always.

I have not studied how unsubscribe works inside and why / how it happens but the weak reference security avoid the effects of this behavior.

Regards

@zsxwing of course I have such call in onDestroy method.

However, I can't reproduce exactly what happens when the application crashes but in my observable (the one having issues) I am executing some shell commands , and in some cases, for some reasons, unsubscribe doesn't kill immediately the working thread. Most of the time it does but not always.

I have not studied how unsubscribe works inside and why / how it happens but the weak reference security avoid the effects of this behavior.

Regards

@mttkay

This comment has been minimized.

Show comment
Hide comment
@mttkay

mttkay Sep 30, 2013

Contributor

Interesting, thanks. This paragraph applies to Android in the same way it applied to the author:

There are some rare scenarios in GUI and some other programming when the unsubscription moment is non-determined and using the strong subscription will cause memory and resources leaks. So it's not possible to explicitly control the unsubscription sometimes

Android may decide non-deterministically that it's time to destroy a "sleeping" Activity, e.g. because the system runs out of memory. I think weak references are an easy and straight forward way to deal with this. The activity is either there or it's not. If it's not, drop the message, as consuming it would crash the app.

It's an interesting thought however to implement this on the Observable level rather than in a decorating observer. This for me falls into the general question that keeps bugging me: which parts of Rx should be implemented as operators and which parts in observers. In this case I found the observer to be the more natural place, since the requested behaviour is purely specific to the subscriber side. I don't really see how an operator that turns an observable sequence into a less "reliable" sequence is very useful outside this case which seems to be specific to GUIs.

Contributor

mttkay commented Sep 30, 2013

Interesting, thanks. This paragraph applies to Android in the same way it applied to the author:

There are some rare scenarios in GUI and some other programming when the unsubscription moment is non-determined and using the strong subscription will cause memory and resources leaks. So it's not possible to explicitly control the unsubscription sometimes

Android may decide non-deterministically that it's time to destroy a "sleeping" Activity, e.g. because the system runs out of memory. I think weak references are an easy and straight forward way to deal with this. The activity is either there or it's not. If it's not, drop the message, as consuming it would crash the app.

It's an interesting thought however to implement this on the Observable level rather than in a decorating observer. This for me falls into the general question that keeps bugging me: which parts of Rx should be implemented as operators and which parts in observers. In this case I found the observer to be the more natural place, since the requested behaviour is purely specific to the subscriber side. I don't really see how an operator that turns an observable sequence into a less "reliable" sequence is very useful outside this case which seems to be specific to GUIs.

@mttkay

This comment has been minimized.

Show comment
Hide comment
@mttkay

mttkay Sep 30, 2013

Contributor

I also noticed that his implementation of a WeakObserver auto-unsubscribes from the observable: http://pastie.org/901863

That could be useful on Android too.

Contributor

mttkay commented Sep 30, 2013

I also noticed that his implementation of a WeakObserver auto-unsubscribes from the observable: http://pastie.org/901863

That could be useful on Android too.

@mttkay

This comment has been minimized.

Show comment
Hide comment
@mttkay

mttkay Sep 30, 2013

Contributor

Actually just talked to a co-worker about this, and it might end up being quite powerful after all to implement these things as an operator (e.g. OperationToUserInterfaceEvents). This would reduce client side boiler plate even further by doing the following things:

  1. Wrap both the subscription and the target observer in an inner observer that keeps weak references
  2. Automatically schedule the observer on the Android UI thread (since this is just boilerplate right now)
  3. Automatically unsubscribe if the reference to the target observer is gone

This still doesn't encompass checking for isAttached or isFinishing, but perhaps that bit could remain in the observer. Thoughts?

Contributor

mttkay commented Sep 30, 2013

Actually just talked to a co-worker about this, and it might end up being quite powerful after all to implement these things as an operator (e.g. OperationToUserInterfaceEvents). This would reduce client side boiler plate even further by doing the following things:

  1. Wrap both the subscription and the target observer in an inner observer that keeps weak references
  2. Automatically schedule the observer on the Android UI thread (since this is just boilerplate right now)
  3. Automatically unsubscribe if the reference to the target observer is gone

This still doesn't encompass checking for isAttached or isFinishing, but perhaps that bit could remain in the observer. Thoughts?

@zsxwing

This comment has been minimized.

Show comment
Hide comment
@zsxwing

zsxwing Sep 30, 2013

Member

@pommedeterresautee Which Scheduler do you use? Does your onNext method run in the UI main thread, or a non-UI thread?

If it runs in a non-UI thread, onNext has a change to run after unsubscribe. For example, this is a snippet of codes in rx.operators.ScheduledObserver.EventLoop which try to invoke your onNext method.

            do {
                Notification<? extends T> notification = queue.poll();
                // if we got a notification, send it
                if (notification != null) {

                    // if unsubscribed stop working
                    if (parentSubscription.isUnsubscribed()) {
                        return parentSubscription;
                    }
                    // process notification

                    switch (notification.getKind()) {
                    case OnNext:
                        underlying.onNext(notification.getValue());
                        break;
                    case OnError:
                        underlying.onError(notification.getThrowable());
                        break;
                    case OnCompleted:
                        underlying.onCompleted();
                        break;
                    default:
                        throw new IllegalStateException("Unknown kind of notification " + notification);
                    }
                }
            } while (counter.decrementAndGet() > 0);

If this part of codes run in a non-UI thread, it may be suspended at switch statement. Then the UI thread runs and invokes the Activity.onDestroy method. Even if unsubscribe is called in the Activity.onDestroy method, when the non-UI thread is back, it still will call the onNext method.

I'm not sure if this is the cause of your issue. Could you check it?

Member

zsxwing commented Sep 30, 2013

@pommedeterresautee Which Scheduler do you use? Does your onNext method run in the UI main thread, or a non-UI thread?

If it runs in a non-UI thread, onNext has a change to run after unsubscribe. For example, this is a snippet of codes in rx.operators.ScheduledObserver.EventLoop which try to invoke your onNext method.

            do {
                Notification<? extends T> notification = queue.poll();
                // if we got a notification, send it
                if (notification != null) {

                    // if unsubscribed stop working
                    if (parentSubscription.isUnsubscribed()) {
                        return parentSubscription;
                    }
                    // process notification

                    switch (notification.getKind()) {
                    case OnNext:
                        underlying.onNext(notification.getValue());
                        break;
                    case OnError:
                        underlying.onError(notification.getThrowable());
                        break;
                    case OnCompleted:
                        underlying.onCompleted();
                        break;
                    default:
                        throw new IllegalStateException("Unknown kind of notification " + notification);
                    }
                }
            } while (counter.decrementAndGet() > 0);

If this part of codes run in a non-UI thread, it may be suspended at switch statement. Then the UI thread runs and invokes the Activity.onDestroy method. Even if unsubscribe is called in the Activity.onDestroy method, when the non-UI thread is back, it still will call the onNext method.

I'm not sure if this is the cause of your issue. Could you check it?

@abersnaze

This comment has been minimized.

Show comment
Hide comment
@abersnaze

abersnaze Sep 30, 2013

Contributor

@mttkay

Which parts of Rx should be implemented as operators and which parts in observers?

All operators are implemented as observers with sprecial behavior. By having them as operators it allows for composition. When I use Rx I generally try to have the final observer be as simple as possible. Often there the body the onNext empty.

Contributor

abersnaze commented Sep 30, 2013

@mttkay

Which parts of Rx should be implemented as operators and which parts in observers?

All operators are implemented as observers with sprecial behavior. By having them as operators it allows for composition. When I use Rx I generally try to have the final observer be as simple as possible. Often there the body the onNext empty.

@mttkay

This comment has been minimized.

Show comment
Hide comment
@mttkay

mttkay Sep 30, 2013

Contributor

Thanks @abersnaze, that sounds sensible. I will set some time aside this or next week to move this behaviour to an operator.

Contributor

mttkay commented Sep 30, 2013

Thanks @abersnaze, that sounds sensible. I will set some time aside this or next week to move this behaviour to an operator.

@pommedeterresautee

This comment has been minimized.

Show comment
Hide comment
@pommedeterresautee

pommedeterresautee Sep 30, 2013

@zsxwing In my application, the Observer is an Activity and so the onNext() method is called on UI thread.

My issue may be related to the fact that I was not checking isFinishing() method as reminded by @mttkay . May be in some cases the Activity is being destroyed when onError() is called, before the unsubscribe inside the onDestroy callback is called, making the application crash.

@mttkay I try use as few as possible Fragment, it introduces complexity in Android application. I use them when I need to have different UI regarding the size / orientation of the screen, but I try to wrap separated task inside dedicated Activities if possible to limit the lots of complex bugs affecting Fragments. However, that's Android stuff not related to Rx, may be not the place to discuss about that.

Regarding the question, where to put stuff, I like the idea to wrap as much as possible things inside a class, hiding checks, auto unsubscribing ... code to the user of the library.

In particular, on Android, most people will have the same needs regarding executing some Async work, no need to bother them more than the strict minimum.

However I don't get why isFinishing / isAttached should be check in the Observer by the lib user? If you have a weak ref to your Observer in your OperationToUserInterfaceEvents class, why not do the job there?

Regards

@zsxwing In my application, the Observer is an Activity and so the onNext() method is called on UI thread.

My issue may be related to the fact that I was not checking isFinishing() method as reminded by @mttkay . May be in some cases the Activity is being destroyed when onError() is called, before the unsubscribe inside the onDestroy callback is called, making the application crash.

@mttkay I try use as few as possible Fragment, it introduces complexity in Android application. I use them when I need to have different UI regarding the size / orientation of the screen, but I try to wrap separated task inside dedicated Activities if possible to limit the lots of complex bugs affecting Fragments. However, that's Android stuff not related to Rx, may be not the place to discuss about that.

Regarding the question, where to put stuff, I like the idea to wrap as much as possible things inside a class, hiding checks, auto unsubscribing ... code to the user of the library.

In particular, on Android, most people will have the same needs regarding executing some Async work, no need to bother them more than the strict minimum.

However I don't get why isFinishing / isAttached should be check in the Observer by the lib user? If you have a weak ref to your Observer in your OperationToUserInterfaceEvents class, why not do the job there?

Regards

@mttkay

This comment has been minimized.

Show comment
Hide comment
@mttkay

mttkay Sep 30, 2013

Contributor

@pommedeterresautee as to your last question. Because the Observer isn't necessarily a fragment or activity, it could be an inner class of either. So you cannot invoke these methods on the observer directly.

Contributor

mttkay commented Sep 30, 2013

@pommedeterresautee as to your last question. Because the Observer isn't necessarily a fragment or activity, it could be an inner class of either. So you cannot invoke these methods on the observer directly.

@pommedeterresautee

This comment has been minimized.

Show comment
Hide comment
@pommedeterresautee

pommedeterresautee Sep 30, 2013

@mttkay I think 90% of users will implement the Observer interface in their Activity/Fragment directly as they do for LoaderManager.LoaderCallbacks when implementing AsyncTaskLoader.

So it may be a good thing to manage both cases, if Observer is an instance of Activity, make all the boring work, otherwise it's up to the lib user to implement these checks in the Observer.

@mttkay I think 90% of users will implement the Observer interface in their Activity/Fragment directly as they do for LoaderManager.LoaderCallbacks when implementing AsyncTaskLoader.

So it may be a good thing to manage both cases, if Observer is an instance of Activity, make all the boring work, otherwise it's up to the lib user to implement these checks in the Observer.

@mttkay

This comment has been minimized.

Show comment
Hide comment
@mttkay

mttkay Oct 9, 2013

Contributor

Could you guys weigh in on this? #427

Thanks!

Contributor

mttkay commented Oct 9, 2013

Could you guys weigh in on this? #427

Thanks!

@benjchristensen

This comment has been minimized.

Show comment
Hide comment
@benjchristensen

benjchristensen Dec 6, 2013

Member

Is work still happening on this issue or shall we close it out?

Member

benjchristensen commented Dec 6, 2013

Is work still happening on this issue or shall we close it out?

@pommedeterresautee

This comment has been minimized.

Show comment
Hide comment
@pommedeterresautee

pommedeterresautee Dec 6, 2013

IMO It can be closed

IMO It can be closed

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