Listview with custom adapter containing CheckBoxes

Question

I have a ListView which uses a custom adapter as shown:

private class CBAdapter extends BaseAdapter implements OnCheckedChangeListener{

    Context context;
    public String[] englishNames;
    LayoutInflater inflater;
    CheckBox[] checkBoxArray;
    LinearLayout[] viewArray;
    private boolean[] checked;

    public CBAdapter(Context con, String[] engNames){
        context=con;
        englishNames=engNames;
        inflater=(LayoutInflater)context.getSystemService(Context.LAYOUT_INFLATER_SERVICE);
        checked= new boolean[englishNames.length];
        for(int i=0; i<checked.length; i++){
            checked[i]=false;
            //Toast.makeText(con, checked.toString(),Toast.LENGTH_SHORT).show();
        }
        checkBoxArray = new CheckBox[checked.length];
        viewArray = new LinearLayout[checked.length];
    }

    public int getCount() {
        return englishNames.length;
    }

    public Object getItem(int position) {
        // TODO Auto-generated method stub
        return null;
    }

    public long getItemId(int position) {
        // TODO Auto-generated method stub
        return 0;
    }

    public View getView(int position, View convertView, ViewGroup parent) {

        if(viewArray[position] == null){

            viewArray[position]=(LinearLayout)inflater.inflate(R.layout.record_view_start,null);

            TextView tv=(TextView)viewArray[position].findViewById(R.id.engName);
            tv.setText(englishNames[position]);

            checkBoxArray[position]=(CheckBox)viewArray[position].findViewById(R.id.checkBox1);
        }

        checkBoxArray[position].setChecked(checked[position]);
        checkBoxArray[position].setOnCheckedChangeListener(this);
        return viewArray[position];
    }


    public void checkAll(boolean areChecked){
        for(int i=0; i<checked.length; i++){
            checked[i]=areChecked;
            if(checkBoxArray[i] != null)
                checkBoxArray[i].setChecked(areChecked);
        }
        notifyDataSetChanged();
    }

    public void onCheckedChanged(CompoundButton cb, boolean isChecked) {
        for(int i=0; i<checked.length; i++){
            if(cb == checkBoxArray[i])
                checked[i]=isChecked;
        }




    }
    public boolean itemIsChecked(int i){
        return checked[i];
    }

}

The layouts are fairly simple so I won't post them unless anyone thinks they are relevant.

The problem is that some of the CheckBoxes are not responding. It seems to be the ones that are visible when the layout is first displayed. Any that you have to scroll down to work as expected.

Any pointers appreciated.

Solution 1

Your code from the answer works but is inefficient(you can actually see this, just scroll the ListView and check the Logcat to see the garbage collector doing it's work). An improved getView method which will recycle views is the one below:

@Override
public View getView(int position, View convertView, ViewGroup parent) {
     LinearLayout view = (LinearLayout) convertView;
     if (view == null) {
          view = (LinearLayout) inflater.inflate(R.layout.record_view_start, parent, false);
     }
     TextView tv = (TextView) view.findViewById(R.id.engName);
     tv.setText(getItem(position));
     CheckBox cBox = (CheckBox) view.findViewById(R.id.checkBox1);
     cBox.setTag(Integer.valueOf(position)); // set the tag so we can identify the correct row in the listener
     cBox.setChecked(mChecked[position]); // set the status as we stored it        
     cBox.setOnCheckedChangeListener(mListener); // set the listener    
     return view;
}

OnCheckedChangeListener mListener = new OnCheckedChangeListener() {

     public void onCheckedChanged(CompoundButton buttonView, boolean isChecked) {   
         mChecked[(Integer)buttonView.getTag()] = isChecked; // get the tag so we know the row and store the status 
     }
};

Regarding your code from your question, at first I thought it was wrong because of the way you setup the rows but I don't see why the adapter will have that behavior as you detached the row view from the list. Also, I even tested the code and it works quite well regarding CheckBoxes(but with very poor memory handling). Maybe you're doing something else that makes the adapter to not work?

Solution 2

Let me first say that you have thrown away one of the main benefits of using an adapter: Reusable views. Holding a hard reference to each created View holds a high risk of hitting the memory ceiling. You should be reusing convertView when it is non-null, and creating your view when convertView is null. There are many tutorials around which show you how to do this.

Views used in an adapter typically have an OnClickListener attached to them by the parent View so that you can set a OnItemClickListener on the ListView. This will supersede any touch listeners on the individual views. Try setting android:clickable="true" on the CheckBox in XML.

Comments

[s1ni5t3r]

Thanks for the heads up about convertView. I had been wondering what this parameter was for. I think I may have a solution to the original problem but I'm still testing. I'll post again soon.

[user]

There are a lot of question on stackoverflow regarding a ListView with a custom adapter that uses a row with a CheckBox, you should look more at those solutions. Your code works but it's very inefficient, each time the ListView is scrolled you'll create new views.

[s1ni5t3r]

I totally agree Luksprog but after many attempts at this, using solutions that work for others, I can't find a solution that works other than this. Hopefully garbage collection will keep up with wasted views and not cause a memory problem and older phones will not be slowed too much. In my app, in most situations, the listview will not have too many entries so scrolling should be kept to a minimum.

[user]

The reason why your code works now is because you inflate the row each time and you avoid the OnCheckedChangeListener messing up the CheckBox status. You should never ignore view recycling in the getView method. Here is your improved getView method that should do what you want but will also recycle views. gist.github.com/3917222

[s1ni5t3r]

Excellent. That works exactly as intended. Your code dropped straight in with only the 'm' removed from the "checked" array. (This is my first project in java and I wasn't aware of the conventions.) I now have 2 favors to ask. 1. Can you repost this as an answer so that I can set it as accepted and 2. Can you explain exactly why this works when my code, in the original post, didn't. I have a feeling that the answer may shed light on some other concepts that I'm struggling with.

[RicNjesh]

How do you do this for a EcxpandableListView with child checkboxes?

[user]

@RicNjesh You'll want to use the getChildView() method which is called whenever a child row is required.

[Muneem Habib]

what is mchecked in this case and how to declare it?

[user]

@MuneemHabib It's a boolean array that is as big as the number of items in the ListView. Declare it in the constructor of the adapter.

[Muneem Habib]

@Luksprog thanks sir it helped me alot. i have resolved my issue

[Muneem Habib]

one more thing why you are creating new row each time? why you are not utilising convert view? i have used convert view and then create new only checkboxes and it is working f9

[user]

@MuneemHabib How exactly I'm creating a new row each time? There is the view == null check and only then I'm inflating a new row.

[Muneem Habib]

i am talking about textview its been created each time

[user]

@MuneemHabib I'm not creating a TextView anywhere, I'm looking for it in the row view with findViewById().