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

It's not clear if ButterKnife.unbind is required for fragments #291

Closed
lorenzos opened this Issue Jul 2, 2015 · 5 comments

Comments

4 participants
@lorenzos

lorenzos commented Jul 2, 2015

Documentation states:

Fragments have a different view lifecycle than activities. When binding a fragment in onCreateView, set the views to null in onDestroyView.

To me, it's not clear if this is strict requirement or not. I never implemented onDestroyView in my fragments, because the system is supposed to clear view references itself when fragment is destroyed in onDestroy. So:

  1. Does documentation just suggest to do so, in order to force garbage collection of those views (a good practice for someone)?

  2. Does ButterKnife require it because it handles views in such a way that there is a memory leak risk in fragments (eg. Fragment.setRetainInstance or similar)?

@lorenzos lorenzos changed the title from It's not clear if ButterKnife.unbind is needed for fragments or not to It's not clear if ButterKnife.unbind is required for fragments Jul 2, 2015

@egor-n

This comment has been minimized.

Contributor

egor-n commented Jul 2, 2015

There can be a case when fragment's view is destroyed, but the fragment instance is still present. For example, when fragment goes into the back stack - onDestroyView() is called, but onDestroy() is not.

@artem-zinnatullin

This comment has been minimized.

Contributor

artem-zinnatullin commented Jul 2, 2015

"System" will not clear view references in onDestroy(), they're just regular references and they would be freed only if GC will collect them. So for example if you have long running async code with strong reference to the Fragment, GC won't collect bonded view objects until there won't be strong references to the Fragment and you'll have memory leak which would keep not only Fragment itself but also bounded Views.

Calling unbind() in onDestroyView() is not required, but recommended.

But you should also keep in mind that if you don't prevent async callbacks that work with bounded Views after onDestroyView() app could be crashed by NullPointerException because of nulled View references.

@lorenzos

This comment has been minimized.

lorenzos commented Jul 2, 2015

@artem-zinnatullin @egor-n Thanks for the clarifications.

@artem-zinnatullin About the async stuff, sure!, I always check views still exists in my callbacks.

@JakeWharton

This comment has been minimized.

Owner

JakeWharton commented Jul 2, 2015

It is not required for ButterKnife. The code does not care whether or not you do this. As to fragments, I'm not sure if it's strictly required or not as I don't use them and never cared to look. The documentation certainly seems to indicate that it's something you should do.

@JakeWharton JakeWharton closed this Jul 2, 2015

@lorenzos

This comment has been minimized.

lorenzos commented Jul 2, 2015

@JakeWharton Thank you. I'll surely try ButterKnife in the next company's project! 🔪

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