4

I am working where I have checkboxe inside RecyclerView items. I am aware of some of its issues. But, with only 10 items and no scrolling wrong listener is called.

I have something like this:

private class MyAdapter extends RecyclerView.Adapter<MyViewHodler> {
    ArrayList<String> items;
    ArrayList<String> checkedList = new ArrayList<>();
    ...

    public void onBindViewHolder(final MyViewHolder holder, int position) {
        String item = items.get(position);
        String check = checkedList.get(item);
        if(check != null)
            holder.vChecked.setChecked(true);
        else
            holder.vChecked.setChecked(false);

        ....

        holder.vChecked.setOnCheckedChangeListener(new CompoundButton.OnCheckedChangeListener() {
                @Override
                public void onCheckedChanged(CompoundButton compoundButton, boolean isChecked) {
                    if (isChecked) {
                        checkedList.add(item);
                    } else {
                        checkedList.remove(item);
                    }
                }
    }

}

Now, strangely this checkedChange calls are made on different views. I have a list of only 10 items and are fit in the recyclerview, no scrolling. I externally change the data (items and checked list), set the new data and calling myAdapter.notifyDatasetChanged()

This call removes one more item from the checked state.

For example, I have 10 items all checked. I uncheck 1 item, and doing some other calculations based on that and calling myAdapter.notifyDatasetChanged(). It also unchecks one more element from the other 9 items.

So even without scrolling why onCheckedChange listener is called for the wrong view?

kirtan403
  • 7,293
  • 6
  • 54
  • 97

2 Answers2

9

I have found out the problem. I thought onBindViewHolder is called for every view to change the data. But I was wrong.

notifyDatasetChanged() recycles all the view holders on screen and from the recycled view holder pool randomly viewholders are picked to bind the new data. So when I called the notifyDatasetChanged, the viewholder was used at the different position, and thus changed one more item.

So, I removed the listener to null in onViewRecycled method of the recycler view. Like this:

    @Override
    public void onViewRecycled(MyViewHolder holder) {
        super.onViewRecycled(holder);

        holder.vChecked.setOnCheckedChangeListener(null);
    }

This, did solved the issue. I was aware of this, but I thought viewholders are reused for the same position if there is no scrolling. But onDatasetChanged recycles all currently used view holders and uses it again from the random pool.

kirtan403
  • 7,293
  • 6
  • 54
  • 97
1

This line: String item = items.get(item);

It bothers me. Shouldn't it be String item = items.get(position);

Edit: If you change one position of the dataset, call notifyItemChanged. Don't notify the whole dataset unless it has changed entirely. Could be this.

Edit 2: Well then the problem is not that onCheckedChanged is being called on the wrong views. It is called in all of the views because you always change the checkbox state at onBindViewHolder

  • Based on one item I changes some calculations which reflects in all other items. So i need to call notifyDatasetChanged – kirtan403 Jul 23 '16 at 15:20
  • I see. Then that's why ``onCheckedChanged`` is always called. You always change the checkboxes' state at ``onBindViewHolder``, and when you notify the dataset has changed, this method is called once for every item in the RecyclerView. –  Jul 23 '16 at 15:25
  • But it should called on the binded view only. As views are not recycled nor scrolled. And not some other random item.. – kirtan403 Jul 23 '16 at 15:26
  • 1
    Regardless, if ten items are displayed, Ten items are binded to the list. If you call ``notifyDatasetChanged``, ``onBindViewHolder`` will be called for every item. They will be rebound. –  Jul 23 '16 at 15:34
  • Sorry for teh late reply. But, I found what was wrong. I know onBindViewHolder will be called for every item, but just to reinflate the layout with new data. But, I was wrong. I have added an answer – kirtan403 Jul 24 '16 at 05:25