Saving selected RadioButton's text in model class from recyclerview item

Question

From RecyclerView item, I am trying to save the selected RadioButton's text in model class. When I select a RadioButton from 1st item, it's text is saved appropriately. Problem is, RadioButton's text at the same position from 8th item is also auto saved. If I select radio button from 2nd item, text from 9th item is also auto saved, and so on. How to solve this problem?

onBindViewHolder method is given below :

    @Override
    public void onBindViewHolder(@NonNull final ViewHolder holder, final int position) {

    ...

    holder.radioGroup.setTag(position);
    holder.radioGroup.setOnCheckedChangeListener(new RadioGroup.OnCheckedChangeListener() {
        @Override
        public void onCheckedChanged(RadioGroup group, int checkedId) {

            int radioButtonID = group.getCheckedRadioButtonId();
            RadioButton radioButton = (RadioButton) group.findViewById(radioButtonID);
            int clickedPos = (Integer) group.getTag();
            models.get(clickedPos).setChecked(radioButtonID);

            if (radioButtonID > 0){
                models.get(clickedPos).setSelectedAns(radioButton.getText().toString());
            }

        }
    });
    holder.radioGroup.check(models.get(position).getChecked());
    Log.d("TAG", "At position " + position + " selected : " + models.get(position).getSelectedAns());

}

I guess this is a cell recycling issue, use interface instead to pass the position of the item clicked(viewHolder position) to activity where the model will be updated — NIKHIL MAURYA, Oct 26 '18 at 17:13
Shouldn't the model get updated in onCheckedChanged inside onBindViewHolder? — Newaj, Oct 26 '18 at 17:16

score 2 · Accepted Answer · answered Oct 27 '18 at 08:32

The problem is that although the code looks correct on the surface, what's actually happening is that when you call holder.radioGroup.check(), it triggers your onCheckedChanged() event handler just the same as if the user had initiated it.

Since the views are recycled, the view at position 0 is being reused for position 8 in the list. So the call to check() in onBindViewHolder() will call onCheckedChanged(), with the checked radio button from position 0 still checked (i.e. checkedId and radioGroup. getCheckedRadioButtonId() will return the ID of the radiobutton checked when the view was used at position 0).

The real crux is this

models.get(clickedPos).setChecked(radioButtonID);

Consider the first paragraphs of the answer, and you'll realize that this will (incorrectly) update the model item at position 8 with the radioButtonID that was checked when this view was used at position 0.

One way to solve this is to distinguish between a user-initiated change and a binding-initiated change. You can for example do this by adding a field to the ViewHolder to indicate if the view is currently binding.

class ViewHolder extends RecyclerView.ViewHolder{
    TextView selectedAnswer;
    RadioGroup radioGroup;

    boolean isBinding;

    ViewHolder(View itemView) {
        super(itemView);

        radioGroup = itemView.findViewById(R.id.radioGroup);

        radioGroup.setOnCheckedChangeListener(new RadioGroup.OnCheckedChangeListener() {
            @Override
            public void onCheckedChanged(RadioGroup group, int checkedId) {
                int position = getAdapterPosition();

                RadioButton radioButton = (RadioButton) group.findViewById(checkedId);

                /* Only update the model when onCheckedChange() was initiated by the user clicking
                   a radio button, not when the adapter is binding the view. In that scenario, we
                   are only interested in passing information FROM the model TO the view. */
                if( !isBinding ) {
                    models.get(position).setChecked(checkedId);
                    models.get(position).setSelectedAns(radioButton != null ? radioButton.getText().toString() : "");
                }

                selectedAnswer.setText(  models.get(position).getSelectedAns() );
            }
        });

        ...
    }
}

@Override
public void onBindViewHolder(@NonNull final ViewHolder holder, final int position) {
    holder.isBinding = true;

    ...

    /* When calling check() here, we invoke onCheckedChanged(), which will
       update the textview that displays the selected answer - so no need to call
           holder.selectedAnswer.setText(  models.get(position).getSelectedAns() )
       from here */
    holder.radioGroup.check(models.get(position).getChecked());

    holder.isBinding = false;
}

I got a rough solution to the problem. If I use holder.setIsRecyclable(false) inside onBindViewHolder(), then I get the expected result. Will it be a good choice? — Newaj, Oct 27 '18 at 08:41
I would say that it is a bad workaround to use `setIsRecyclable(false)` in this kind of scenario. It will prevent the `RecyclerView` from reusing `ViewHolder`s, which kind of defeats the whole purpose of the `RecyclerView`. Instead, it will create a new `ViewHolder` every time you scroll, which will result in worse performance. The main point of using the `ViewHolder` pattern is to improve performance (specifically avoiding lots of calls to `findViewById()` on scrolling). — Magnus, Oct 27 '18 at 09:02

Saving selected RadioButton's text in model class from recyclerview item

1 Answers1

Linked