Skip to content
This repository has been archived by the owner on Feb 26, 2023. It is now read-only.

Optionally ignore @UIThread call when @EFragment is detached from Activity? #875

Closed
vincentjames501 opened this issue Jan 10, 2014 · 10 comments

Comments

@vincentjames501
Copy link

When using @Background in conjunction with @UIThread annotations on an @EFragment, it's possible that many developers don't want to run the method annotated with the @UIThread annotation if the Fragment is no longer attached to the activity. I think it would be great to add an annotation (something like @IgnoredWhenDetached) with the following rules:

  1. Must also have the @UIThread annotation
  2. Can only be used in conjunction with @EFragment classes
  3. Simply wraps the @UIThreadmethod call in an if block like so
if(isAdded()) {
    //executable code
}

or

if(getActivity() != null) {
    //executable code
}

Here is a simple example and in my application I have dozens of things similar to this

@EFragment
public class LoaderFragment extends Fragment {

    @Background
    void longTask() {
        try {
            updateProgress(0);
            Thread.sleep(1000);
            updateProgress(50);
            Thread.sleep(1000);
            updateProgress(100);
        } catch (InterruptedException e) {
            killActivity()        
        }
    }

    @IgnoredWhenDetached
    @UiThread
    void updateProgress(int progress) {
        getActivity().setProgress(progress);
    }

    @IgnoredWhenDetached
    @UiThread
    void killActivity(int progress) {
        getActivity().finish();
    }

This essentially prevents the two @UIThread methods from getting a NPE if the Fragment were to have been destroyed at any point during the execution of the @Background method. While I realize this is something that can be easily guarded against by a simple null check, it makes the code much cleaner and is a very common use case in conjunction with Fragments. I would be more than happy to submit a pull request pending thoughts/comments about this. This could also be useful in conjunction with @Background on an @EFragment as well, but not a very common use case.

Vince

@WonderCsabo
Copy link
Member

Actually #823 proposed the same, but it was rejected.

@vincentjames501
Copy link
Author

I read his actually before posting this. This is slightly different in that I'm not proposing making it the default behavior of @UIThread for @EFragment classes.

@WonderCsabo
Copy link
Member

Yes, you are right.

@DayS
Copy link
Contributor

DayS commented Jan 11, 2014

Yeah, I prefer this solution as the developer still have the choice to handle theses cases as he want.
This may be in 3.1 :)

@WonderCsabo
Copy link
Member

I tought about the API, and bringed up a version which uses a parameter in the @UiThread and not a new annotation. But i changed my mind since this feature is only for Fragments, @UiThread API should not be affected.
I think you can start to implement this if you want. :)

@yDelouis
Copy link
Contributor

I think we could use this annotation even on methods which are not annotated with @UIThread or @Background.

@vincentjames501
Copy link
Author

I think you may be right. I'll look at doing another pull request then let the authors decide which, if any, they would like to merge.

@DayS
Copy link
Contributor

DayS commented Jan 13, 2014

@yDelouis > Good idea 👍

@toshe
Copy link
Contributor

toshe commented May 17, 2014

This is an extremely good idea!
I'm currently using

if(isAdded()) {
    //executable code
}

in order to avoid the problem when switching really fast through fragments. An annotation like that makes total sense.

@DayS
Copy link
Contributor

DayS commented Jun 8, 2014

Merged. Thanks to @yDelouis and @vincentjames501 for this

@DayS DayS closed this as completed Jun 8, 2014
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants