BaseAdapter causing ListView to go out of order when scrolled

I'm having problems with some BaseAdapter code that I adapted from a book. I've been using variations of this code all over the place in my application, but only just realized when scrolling a long list the items in the ListView become jumbled and not all of the elements are displayed.

It's very hard to describe the exact behavior, but it's easy to see if you take a sorted list of 50 items and start scrolling up and down.

class ContactAdapter extends BaseAdapter {

    ArrayList<Contact> mContacts;

    public ContactAdapter(ArrayList<Contact> contacts) {
        mContacts = contacts;
    }

    @Override
    public int getCount() {
        return mContacts.size();
    }

    @Override
    public Object getItem(int position) {
        return mContacts.get(position);
    }

    @Override
    public long getItemId(int position) {
        return position;
    }

    @Override
    public View getView(int position, View convertView, ViewGroup parent) {
        View view;
        if(convertView == null){
            LayoutInflater li = getLayoutInflater();
            view = li.inflate(R.layout.groups_item, null);
            TextView label = (TextView)view.findViewById(R.id.groups_item_title);
            label.setText(mContacts.get(position).getName());
            label = (TextView)view.findViewById(R.id.groups_item_subtitle);
            label.setText(mContacts.get(position).getNumber());
        }
        else
        {
            view = convertView;
        }
        return view;
    }

}

Solution 1:

You are only putting data in the TextView widgets when they are first created. You need to move these four lines:

        TextView label = (TextView)view.findViewById(R.id.groups_item_title);
        label.setText(mContacts.get(position).getName());
        label = (TextView)view.findViewById(R.id.groups_item_subtitle);
        label.setText(mContacts.get(position).getNumber());

to be after the if/else block and before the method return, so you update the TextView widgets whether you are recycling the row or creating a fresh one.

Solution 2:

To further clarify the answer of CommonsWare, here is some more info:

The li.inflate operation (needed here for parsing of the layout of a row from XML and creating the appropriate View object) is wrapped by an if (convertView == null) statement for efficiency, so the inflation of the same object will not happen again and again every time it pops into view.

HOWEVER, the other parts of the getView method are used to set other parameters and therefore should NOT be included within the if (convertView == null){ }... else{ } statement.

In many common implementation of this method, some textView label, ImageView or ImageButton elements need to be populated by values from the list[position], using findViewById and after that .setText or .setImageBitmap operations. These operations must come after both creating a view from scratch by inflation and getting an existing view if not null (e.g. on a refresh).

Another good example where this solution is applied for a ListView ArrayAdapter appears in https://stackoverflow.com/a/3874639/978329