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

Add @IgnoreWhenViewDestroyed #1597

Closed
Closed
Add @IgnoreWhenViewDestroyed#1597
Labels
Enhancement

Description

@WonderCsabo
WonderCsabo
opened on Oct 20, 2015

Now AndroidAnnotations clears injected view fields in onDestroyView() by setting each to null. However background tasks can be completed later. To avoid NPE-s, currently the client has to check wether the views are null or not. We could create a new annotation just like @IgnoreWhenDetached to remove that boilerplate. This was already suggested by @nbelikov.

Activity

  1. WonderCsabo commented on Oct 20, 2015

    @WonderCsabo
    WonderCsabo
    on Oct 20, 2015
    MemberAuthor

    @yDelouis @dodgex WDYT?

  2. WonderCsabo commented on Oct 20, 2015

    @WonderCsabo
    WonderCsabo
    on Oct 20, 2015
    MemberAuthor

    Or maybe we could generalize the @IgnoreWhenXXX annotations, like @IgnoreWhen(DETACHED) or @IgnoreWhen(VIEW_DESTROYED). I am not sure.

  3. dodgex commented on Oct 20, 2015

    @dodgex
    dodgex
    on Oct 20, 2015
    Member

    i'd prefer the @IgnoreWhen(XXX) approach.

  4. yDelouis commented on Oct 21, 2015

    @yDelouis
    yDelouis
    on Oct 21, 2015
    Contributor

    What would be the generated code ?

  5. WonderCsabo commented on Oct 21, 2015

    @WonderCsabo
    WonderCsabo
    on Oct 21, 2015
    MemberAuthor

    I thought about the following:

    boolean viewDestroyed;

View onCreateView() {
  ...
  viewDestroyed = false;
}

void onDestroyView() {
  ...
  viewDestroyed = true;
}

void methodIgnoredWhenViewDestroyed() {
  if (!viewDestroyed) {
    super.methodIgnoredWhenViewDestroyed();
  }
}

  6. yDelouis commented on Oct 21, 2015

    @yDelouis
    yDelouis
    on Oct 21, 2015
    Contributor

    Okay. Why not.

  7. WonderCsabo commented on Oct 21, 2015

    @WonderCsabo
    WonderCsabo
    on Oct 21, 2015
    MemberAuthor

    So can we rename the @IgnoreWhenDetached annotation to @IgnoreWhen and add an option to it?

  8. yDelouis commented on Oct 21, 2015

    @yDelouis
    yDelouis
    on Oct 21, 2015
    Contributor

    Yes.

  9. WonderCsabo
    added
    Enhancement
    on Oct 21, 2015
  10. WonderCsabo
    self-assigned this
    on Oct 21, 2015

  11. shiraji commented on Dec 17, 2015

    @shiraji
    shiraji
    on Dec 17, 2015
    Contributor

    I am taking care of this one. Here is TODO

    • Create @IgnoreWhen that takes DETACHED or VIEW_DESTROYED (or DESTROY_VIEW?)
    • Based on the parameter, viewDestroyed is set to true at onDetach or onDestroyView
    • New annotation is accepted the method that has @UiThread annotation.

  12. WonderCsabo commented on Dec 17, 2015

    @WonderCsabo
    WonderCsabo
    on Dec 17, 2015
    MemberAuthor

    I vote for VIEW_DESTROYED.

    viewDestroyed is only needed if VIEW_DESTROYED is used. For DETACHED, we can use the old method (getActivity() == null), there is no need to create a variable.

    @UiThread is not mandatory for this.

  13. WonderCsabo
    removed their assignment
    on Dec 17, 2015

  14. shiraji commented on Dec 17, 2015

    @shiraji
    shiraji
    on Dec 17, 2015
    Contributor

    Could you check if I understand this issue correctly?

    Generated Code

    @IgnoreWhen(DETACHED)

    void methodIgnoredWhenViewDestroyed() {
  if (getActivity() != null) {
    super.methodIgnoredWhenViewDestroyed();
  }
}

    @IgnoreWhen(VIEW_DESTROYED)

    boolean viewDestroyed;

View onCreateView() {
  ...
  viewDestroyed = false;
}

void onDestroyView() {
  ...
  viewDestroyed = true;
}

void methodIgnoredWhenViewDestroyed() {
  if (!viewDestroyed) {
    super.methodIgnoredWhenViewDestroyed();
  }
}

    TODO

    • Create @IgnoreWhen that takes DETACHED or VIEW_DESTROYED
    • Based on the parameter, generated codes are above.
    • New annotation is acceptable if there is at least one more annotation.

    Quetion

    • No default value for this annotation?
    • Is DETACHED required to put inside Fragment not Activity?
    • Is VIEW_DESTROYED required to put inside Fragment or Activity?

  15. WonderCsabo commented on Dec 17, 2015

    @WonderCsabo
    WonderCsabo
    on Dec 17, 2015
    MemberAuthor

    In onCreateView(), you should set the variable to false ( i guess that is a copy paste error).
    Also, this variable should be volatile (since the decorated method maybe does not run on the main thread).

    New annotation is acceptable if there is at least one more annotation

    What do you mean?

    There is no default value.
    This annotation can be only used in @EFragments.

  16. dodgex commented on Dec 17, 2015

    @dodgex
    dodgex
    on Dec 17, 2015
    Member

    New annotation is acceptable if there is at least one more annotation

    What do you mean?

    i think he means that the new @IgnoreWhen annotations requires at least one other annotation like @UiThread. but i'd say no. this annotation should also work. a method annotated with this could be called from any thread/method so it should be ensured that it always work

  17. shiraji commented on Dec 17, 2015

    @shiraji
    shiraji
    on Dec 17, 2015
    Contributor

    Oh I got it. Thanks @dodgex

    I'm not sure about this

    In onCreateView(), you should set the variable to false ( i guess that is a copy paste error).

    private volatile boolean viewDestroyed;

View onCreateView() {
  ...
  viewDestroyed = false;
}

void onDestroyView() {
  ...
  viewDestroyed = true;
}

void methodIgnoredWhenViewDestroyed() {
  if (!viewDestroyed) {
    super.methodIgnoredWhenViewDestroyed();
  }
}

  18. WonderCsabo commented on Dec 17, 2015

    @WonderCsabo
    WonderCsabo
    on Dec 17, 2015
    MemberAuthor

    @shiraji this last snippet is correct.

  19. shiraji commented on Dec 17, 2015

    @shiraji
    shiraji
    on Dec 17, 2015
    Contributor

    Cool! Thanks for reviewing requirement, guys.

  20. shiraji commented on Dec 17, 2015

    @shiraji
    shiraji
    on Dec 17, 2015
    Contributor

    What about enum name?

    @IgnoreWhen(After.DETACHED)
    @IgnoreWhen(Timing.DETACHED)
    @IgnoreWhen(IgnoreWhen.DETACHED)

    I didn't come up with good one...

  21. dodgex commented on Dec 17, 2015

    @dodgex
    dodgex
    on Dec 17, 2015
    Member

    what about @IgnoreWhen(IgnoreWhen.State.DETACHED) or short @IgnoreWhen(State.DETACHED)

  22. dodgex commented on Dec 17, 2015

    @dodgex
    dodgex
    on Dec 17, 2015
    Member

    having

    @interface IgnoreWhen {

  //code

  enum State {
    // fields
  }
}

  23. shiraji commented on Dec 17, 2015

    @shiraji
    shiraji
    on Dec 17, 2015
    Contributor

    I like short one!

  24. dodgex commented on Dec 17, 2015

    @dodgex
    dodgex
    on Dec 17, 2015
    Member

    you can do both, but if you for example have another State class you might want to use the "long" version.

  25. shiraji commented on Dec 17, 2015

    @shiraji
    shiraji
    on Dec 17, 2015
    Contributor

    You are right. I will put enum inside the annotation.

  26. WonderCsabo commented on Dec 17, 2015

    @WonderCsabo
    WonderCsabo
    on Dec 17, 2015
    MemberAuthor

    Indeed, we should do the same as with EBean.Scope.

  27. shiraji commented on Dec 17, 2015

    @shiraji
    shiraji
    on Dec 17, 2015
    Contributor

    Yea. I will avoid conflict.

  28. WonderCsabo commented on Dec 20, 2015

    @WonderCsabo
    WonderCsabo
    on Dec 20, 2015
    MemberAuthor

    Implemented.

  29. WonderCsabo
    closed this as completedon Dec 20, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    Enhancement

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions