Skip to content
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

Clarify bind/unbind behavior. #96

Closed
awenger opened this issue Jan 17, 2017 · 3 comments
Closed

Clarify bind/unbind behavior. #96

awenger opened this issue Jan 17, 2017 · 3 comments

Comments

@awenger
Copy link

awenger commented Jan 17, 2017

I observed that unbind is not always called before a view is rebound to another model (recycled). I assume this is working as intended and not a bug. However I think the documentation on this behavior could be improved.

Knowing the described behavior I can interpret this information into the documentation:

/**
* Binds the current data to the given view. You should bind all fields including unset/empty
* fields to ensure proper recycling.
*/
public void bind(T view) {
}
/**
* Similar to {@link #bind(Object)}, but provides a non null, non empty list of payloads
* describing what changed. This is the payloads list specified in the adapter's notifyItemChanged
* method. This is a useful optimization to allow you to only change part of a view instead of
* updating the whole thing, which may prevent unnecessary layout calls. If there are no payloads
* then {@link #bind(Object)} is called instead.
*/
public void bind(T view, List<Object> payloads) {
bind(view);
}
/** Subclasses can override this if their view needs to release resources when it's recycled. */
public void unbind(T view) {
}

https://github.com/airbnb/epoxy/blob/c2bdcd29d8d6acce0d47a0a5f7c14b3b4f07d937/README.md

However without this knowledge this was absolutely not obvious for me from the documentation. Especially because of the naming of the two methods bind/unbind and the observed behavior in most cases (most of the time unbind is called before the view is rebound).

@elihart
Copy link
Contributor

elihart commented Jan 20, 2017

You're right, working as intended but still a bit confusing.

I believe that the Epoxy documentation does make sense in the context of RecyclerView. You said before a view is rebound to another model (recycled), but that isn't actually the definition of recycled. Epoxy uses the RecylerView method onViewRecycled which documents the method as

Called when a view created by this adapter has been recycled.

A view is recycled when a RecyclerView.LayoutManager decides that it no longer needs to be attached to its parent RecyclerView. This can be because it has fallen out of visibility or a set of cached views represented by views still attached to the parent RecyclerView. If an item view has large or expensive data bound to it such as large bitmaps, this may be a good place to release those resources.

RecyclerView calls this method right before clearing ViewHolder's internal data and sending it to RecycledViewPool. This way, if ViewHolder was holding valid information before being recycled, you can call RecyclerView.ViewHolder.getAdapterPosition() to get its adapter position.

So the view is only recycled when it goes off screen. It is not recycled when it is rebound to the same model or another model on screen.

The unbind documentation is pretty clear that it is called when the view is recycled Subclasses can override this if their view needs to release resources when it's recycled.. I could have named this method onViewRecycled or something to make that more clear. I liked the symmetry and simplicity of bind/unbind, but it is misleading.

I think it's a bit late now to rename the unbind method, but I can update the javadoc and readme to be more explicit. Thanks for bringing this to my attention!

@awenger
Copy link
Author

awenger commented Jan 24, 2017

Thank you for the clarification about when a View is recycled.

So the view is only recycled when it goes off screen. It is not recycled when it is rebound to the same model or another model on screen.

Exactly this is the problematic part that caused my confusion about when a view is recycled. In most of the cases RecyclerView still recycles a View before it rebinds it to another ViewHolder (At least in my tests). The following (constructed) sample demonstrates this behavior:

public class MainActivity extends AppCompatActivity {

    int i = 0;
    // with this delay unbind is always called. Around 200-300 it stops to call unbind and calls bind directly 
    private final long UPDATE_DELAY = 400;

    @Override
    protected void onCreate(Bundle savedInstanceState) {
        super.onCreate(savedInstanceState);
        setContentView(R.layout.activity_main);

        RecyclerView list = (RecyclerView) findViewById(R.id.list);
        list.setLayoutManager(new LinearLayoutManager(this));

        final TestAdapter adapter = new TestAdapter();

        list.setAdapter(adapter);

        final Handler h = new Handler(Looper.getMainLooper());
        h.postDelayed(new Runnable() {
            @Override
            public void run() {
                adapter.update("Val is : " + (i++));
                h.postDelayed(this, UPDATE_DELAY);
            }
        }, 1000);
    }

    public class TestAdapter extends EpoxyAdapter {

        public TestAdapter() {
            enableDiffing();
        }

        public void update(String val) {
            models.clear();
            models.add(new TestModel(1l, val));
            notifyModelsChanged();
        }
    }

    public class TestModel extends EpoxyModel<LinearLayout> {

        private final String   val;
        private       TextView output;

        public TestModel(long id, String val) {
            super(id);
            this.val = val;
        }

        @Override
        protected int getDefaultLayout() {
            return R.layout.list;
        }

        @Override
        public int hashCode() {
            return super.hashCode() + 31 * val.hashCode();
        }

        @Override
        public void bind(LinearLayout view) {
            super.bind(view);
            output = new TextView(view.getContext());
            output.setText(val);
            view.addView(output);
        }

        // 
        @Override
        public void unbind(LinearLayout view) {
            super.unbind(view);
            // this reverts the view into the initial state as long as the changes are not too frequent.
            view.removeView(output);
        }
    }
}

To keep the TextView in the Model is of course not advisable and is only used to demonstrate the behavior. However, people will add TextWatchers to a view/model, and will look for a way to remove them again. Furthermore unbind will look very promising for AsyncTasks that should be canceled or RxJava Subscriptions that need to be unsubscribed.
This is why I think there should be a explicit warning in the documentation of unbind. Currently the only hint is the keyword recycled:

Subclasses can override this if their view needs to release resources when it's recycled.

@elihart
Copy link
Contributor

elihart commented Jan 31, 2017

#103

@elihart elihart closed this as completed Jan 31, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants