Strange behaviour of checkBox in the listView

Question

I have fragment with a listView, the listview layout contains (TextView, checkBox and ImageView as follows

t1........CB.......IV
t2........CB.......IV
t3........CB.......IV
t4........CB.......IV
t5........CB.......IV
t6........CB.......IV
      SaveButton

in the getView() method, I check if the checkbox is checked or not, and if it is checked, I add the checked items to a list.

the problem is when, initially I set all the checkboxes to be unchecked, but when i check the first item, surprisingly, the third item from below is automatically checked, and if i check the 2nd item from above, the 2nd item from below is checked, and if i check the 3rd item from above , then the last item is checked automatically?!!

please have a look at getView() method and let me know what I missed:

getView():

@Override
public View getView(final int position, View convertView, ViewGroup parent) {
    // TODO Auto-generated method stub

    if (convertView == null) {
        LayoutInflater layoutinflater = (LayoutInflater) context.getSystemService(Activity.LAYOUT_INFLATER_SERVICE);
        convertView = layoutinflater.inflate(R.layout.list_items_layout, null);
    }

    if (this.checkedItemsList == null) {
        this.checkedItemsList = new ArrayList<String>();
    }

    final TextView tv = (TextView) convertView.findViewById(R.id.tvlist_topic);
    final CheckBox cb = (CheckBox) convertView.findViewById(R.id.cbList_hook);
    final ImageView iv = (ImageView) convertView.findViewById(R.id.ivList_delete);

    tv.setText(this.topicsList.get(position));

    cb.setOnCheckedChangeListener(new OnCheckedChangeListener() {

        @Override
        public void onCheckedChanged(CompoundButton buttonView, boolean isChecked) {
            // TODO Auto-generated method stub
            if (isChecked) {
                checkedItemsList.add(topicsList.get(position));
                setCheckedItemsList(checkedItemsList);
                Log.d(TAG, "size: " + checkedItemsList.size());
            }
        }
    });

    iv.setOnClickListener(new OnClickListener() {

        @Override
        public void onClick(View v) {
            // TODO Auto-generated method stub
            if (cb.isChecked())
                cb.setChecked(false);

            topicsList.remove(position);
            notifyDataSetChanged();
        }
    });

    return convertView;
}

xml:

<RelativeLayout 
    android:id="@+id/rl2_List"
    android:layout_width="wrap_content"
    android:layout_height="wrap_content"
    android:layout_toEndOf="@id/rl1_List"
    android:layout_centerInParent="true">
    <CheckBox
        android:id="@+id/cbList_hook"
        android:layout_width="wrap_content"
        android:layout_height="wrap_content"
        android:focusable="false"
        android:focusableInTouchMode="false"
        android:checked="false"/>
</RelativeLayout>

<RelativeLayout 
    android:id="@+id/rl3_List"
    android:layout_width="wrap_content"
    android:layout_height="wrap_content"
    android:layout_toEndOf="@id/rl2_List"
    android:layout_alignParentEnd="true"
    android:layout_marginStart="100dp">
    <ImageView 
        android:id="@+id/ivList_delete"
        android:layout_width="wrap_content"
        android:layout_height="wrap_content"
        android:focusable="false"
        android:focusableInTouchMode="false"
        android:clickable="true"
        android:src="@drawable/delete_icon"
        android:contentDescription="icon to delete item from the Listview"/>
</RelativeLayout>

Answer 1

You have to do something like this

@Override
public View getView(final int position, View convertView, ViewGroup parent) {
// TODO Auto-generated method stub

if (convertView == null) {
    LayoutInflater layoutinflater = (LayoutInflater) context.getSystemService(Activity.LAYOUT_INFLATER_SERVICE);
    convertView = layoutinflater.inflate(R.layout.list_items_layout, null);
}

if (this.checkedItemsList == null) {
    this.checkedItemsList = new ArrayList<String>();
}

final TextView tv = (TextView) convertView.findViewById(R.id.tvlist_topic);
final CheckBox cb = (CheckBox) convertView.findViewById(R.id.cbList_hook);
final ImageView iv = (ImageView) convertView.findViewById(R.id.ivList_delete);

tv.setText(this.topicsList.get(position));
cb.setTag(position);
cb.setOnCheckedChangeListener(new OnCheckedChangeListener() {

    @Override
    public void onCheckedChanged(CompoundButton buttonView, boolean isChecked) {
        // TODO Auto-generated method stub
        if (isChecked) {
            checkedItemsList.add(topicsList.get(Integer.parseInt(buttonView.getTag().toString())));
            setCheckedItemsList(checkedItemsList);
            Log.d(TAG, "size: " + checkedItemsList.size());
        }
    }
});



return convertView;
}

Answer 2

It would be better if you use the ViewHolder pattern. It will prevent the overhead of inflating the views every time a user scrolls through the list. Also whether a checkbox is checked or not should be present in the data source (the data structure you are using to fill the ListView; in this case topicsList). Assuming the model class of the data-structure is named Model:

public class Model {
 private boolean mChecked = false;

 public boolean isChecked() {
  return this.mChecked;
 }
 public void setChecked(boolean checked) {
  this.mChecked = checked;
 }
}

Now in your ListView's adapter class

public class SomeAdapter extends BaseAdapter {

 private ArrayList<Model> topicsList; //assuming this is filled with data
 static class ViewHolder { //the view holder's class
  TextView text;
  CheckBox checkBox;
  ImageView imageView;
 }


 @Override
 public View getView(final int position, View convertView, ViewGroup parent) {
    // TODO Auto-generated method stub
    ViewHolder holder;
    if (convertView == null) {
        LayoutInflater layoutinflater = (LayoutInflater) context.getSystemService(Activity.LAYOUT_INFLATER_SERVICE);
        convertView = layoutinflater.inflate(R.layout.list_items_layout, null);
        TextView tv = (TextView) convertView.findViewById(R.id.tvlist_topic);
        CheckBox cb = (CheckBox) convertView.findViewById(R.id.cbList_hook);
        ImageView iv = (ImageView) convertView.findViewById(R.id.ivList_delete);
        holder = new ViewHolder(); 
        holder.textView = tv;
        holder.checkBox = cb;
        holder.imageView = iv;
        convertView.setTag(viewHolder);
    } else {
        viewHolder = (ViewHolder) converView.getTag();
    }

    if (this.checkedItemsList == null) {
        this.checkedItemsList = new ArrayList<String>();
    }


    TextView textView = viewHolder.textView;
    CheckBox checkBox = viewHolder.checkBox;
    ImageView imageView = viewHolder.imageView;    

    textView.setText(this.topicsList.get(position));
    if (this.topicsList.get(position).isChecked()) {
     checkBox.setChecked(true);
    } else {
     checkBox.setChecked(false);
    }

    checkBox.setOnCheckedChangeListener(new OnCheckedChangeListener() {

        @Override
        public void onCheckedChanged(CompoundButton buttonView, boolean isChecked) {
            // TODO Auto-generated method stub
            if (isChecked) {
                SomeAdapter.this.topicsList.get(position).setChecked(isChecked);
              checkedItemsList.add(SomeAdapter.this.topicsList.get(position));
                setCheckedItemsList(checkedItemsList);
                Log.d(TAG, "size: " + checkedItemsList.size());
            }
        }
    });

    imageView.setOnClickListener(new OnClickListener() {

        @Override
        public void onClick(View v) {
            // TODO Auto-generated method stub
            if (checkBox.isChecked()) {
                checkBox.setChecked(false);
                SomeAdapter.this.topicsList.get(position).setChecked(false);
                topicsList.remove(position);
                notifyDataSetChanged();
            } else { //if needed
                checkBox.setChecked(true); 
                SomeAdapter.this.topicsList.get(position).setChecked(true);
                //perform other operations here
            }

        }
    });

    return convertView;
 }
}

Answer 3

You need to learn about convertView, it is often more confusing for beginners.

For example, if you have 100 Checkboxes in a huge list, but only 5 are visible on the screen at once, the naive way is to create 100 objects for Checkbox(indeed this is how it worked in earlier versions of Android). However, creating new objects costs CPU not only during creation, but also during Garbage Collection. That is why a convertView is used. Android system will create only 5 objects and reuse the same objects as the user scrolls. In your case, I think only 3 objects are on screen. So, technically, the first object that scrolled out of screen and the fourth object that shows up on screen are the same one.

The solution for you is to treat the View objects as visual elements and save the state else where. I think the other answer suggests code, so, I'll refrain from suggesting code and encourage you to understand the concept and figure out a solution yourself.

Answer 4

You have to persist your objects checked state.

I would implement an instance variable in your object (in topicsList) which indicates if it's checked or not.

Something like

public class Topic{

public boolean isChecked;

... //Other instance variables

public boolean isChecked() {
    return isChecked;
}

public void setChecked(boolean isChecked) {
    this.isChecked = isChecked;
}

...
//other getters and setters

}

Then in your listAdapter getView method, you check this value and set the checkbox accordingly.

final CheckBox cb = (CheckBox) convertView.findViewById(R.id.cbList_hook);

boolean isChecked = topicsList.get(position).isChecked();

cb.setChecked(isChecked);

//Set a tag on the cb, to know which object it corresponds with
cb.setTag(topicsList.get(position));

Also, you have to change your objects checked value when it's clicked.

cb.setOnCheckedChangeListener(new OnCheckedChangeListener() {

    @Override
    public void onCheckedChanged(CompoundButton buttonView, boolean isChecked) {
        // TODO Auto-generated method stub
        if (isChecked) {
            checkedItemsList.add(topicsList.get(position));
            setCheckedItemsList(checkedItemsList);
            Log.d(TAG, "size: " + checkedItemsList.size());
        }
        CheckBox cb = (CheckBox) buttonView;
        Object yourObject = (Object) buttonView.getTag();
        yourObject.setChecked(isChecked);
    }
});