moonlightcheese
moonlightcheese

Reputation: 10590

Android CursorAdapter drawing things in the wrong list item

I’ve got a CursorAdapter that's built from a database Cursor and if a View in the first item isn't filled in, it populates the first item's Views with whatever data appears in that View in the first subsequent list item where the View was set. In addition, I find that onBindView gets triggered three times for each list item, which seems odd. and it only does this for the first list item, not all the items with null Views. Here’s what it looks like:

example

Note that the address of "Julian" got copied to "Alice". whenever I set a break point in the onBindView method, I can see that when the customerName is "Alice Martin", if(billingAddressCursor!=null && billingAddressCursor.getCount()>0) always evaluates to false for all three calls to onBindView, so I know it never goes into the billing conditional.

Why is it drawing the wrong values in the first position and how can I stop it? code:

private class CustomerAdapter extends CursorAdapter {
    /*** DEBUG CODE, TO BE REMOVED ***/
    int aliceBindCount = 0;
    int blakeBindCount = 0;
    int cscBindCount = 0;
    int julianBindCount = 0;
    /*** ^^^^^^^^^^^^^^^^^^^^^^^^^ ***/
    public CustomerAdapter(Context context, Cursor cursor) {
        super(context, cursor);
    }

    public void bindView(View convertView, Context context, Cursor cursor) {
        if(convertView!=null) {
            String customerName = cursor.getString(cursor.getColumnIndex(CustomerSchema.NAME));
            ((TextView)convertView.findViewById(R.id.cli_customer_name)).setText(customerName);
            /*** DEBUG CODE, TO BE REMOVED ***/
            if(customerName.equals("Alice Martin")) {
                aliceBindCount += 1;
            } else if(customerName.equals("Blake Slappey")) {
                blakeBindCount += 1;
            } else if(customerName.equals("Conceptual Systems")) {
                cscBindCount += 1;
            } else if(customerName.equals("Julian")) {
                julianBindCount += 1;
            }
            /*** ^^^^^^^^^^^^^^^^^^^^^^^^^ ***/
        }
        final Long billingAddressID = cursor.getLong(cursor.getColumnIndex(CustomerSchema.BILLING_ADDRESS_ID));
        Cursor billingAddressCursor = DbDesc.getInstance().getDatabase(getActivity()).query(
                LocationSchema.TABLE_NAME,
                null,
                LocationSchema._ID+"=?",
                new String[]{ String.valueOf(billingAddressID) },
                null,
                null,
                null
        );
        if(billingAddressCursor!=null && billingAddressCursor.getCount()>0) {
            billingAddressCursor.moveToFirst();
            String street = billingAddressCursor.getString(billingAddressCursor.getColumnIndex(LocationSchema.STREET));
            String city = billingAddressCursor.getString(billingAddressCursor.getColumnIndex(LocationSchema.CITY));
            String state = billingAddressCursor.getString(billingAddressCursor.getColumnIndex(LocationSchema.STATE));
            String zip = billingAddressCursor.getString(billingAddressCursor.getColumnIndex(LocationSchema.ZIP));
            if(zip==null || zip.equals("")) {
                zip = "[NO ZIP]";
            }
            if(street==null || street.equals("")) {
                street = "[NO STREET]";
            }
            if(city==null || city.equals("")) {
                city = "[NO CITY]";
            }
            if(state==null || state.equals("")) {
                state = "[NO STATE]";
            }
            ((TextView)convertView.findViewById(R.id.cli_street)).setText(street);
            ((TextView)convertView.findViewById(R.id.cli_city_state_zip)).setText(city+", "+state+" "+zip);
        }
    }

    public View newView(Context context, Cursor cursor, ViewGroup parent) {
        LayoutInflater inflater = LayoutInflater.from(context);
        View v = inflater.inflate(R.layout.customer_list_item, parent, false);
        return v;
    }
}

And I am certain that Alice should not have a billing address associated with that record. Here is the table query from sqlite3. The only record with a billing address id is Julian's record, as you can see:

sqlite> .tables
.tables
android_metadata  contract          location
contact           customer          phone
sqlite> select * from customer;
select * from customer;
2|Conceptual Systems|||||||
3|Blake Slappey|||||||
4|Julian|1||||||
5|Alice Martin|||||||

Upvotes: 2

Views: 839

Answers (2)

Gene
Gene

Reputation: 46960

ListView recycles its row views, so old values that aren't explicitly set will hang around in subsequent uses. In your code, if the query inside your binder doesn't return any rows, the values in the view from its previous use will remain. Not what you want.

Here is a re-coding that should fix the problem:

   String street = "[NO STREET]";
   String city = "[NO CITY]";
   String state = "[NO STATE]";
   String zip = "[NO ZIP]";
   if(billingAddressCursor!=null && billingAddressCursor.getCount()>0) {
        billingAddressCursor.moveToFirst();
        String tmp = billingAddressCursor.getString(billingAddressCursor.getColumnIndex(LocationSchema.STREET));
        if (tmp != null) street = tmp;
        tmp = billingAddressCursor.getString(billingAddressCursor.getColumnIndex(LocationSchema.CITY));
        if (tmp != null) city = tmp;
        tmp = billingAddressCursor.getString(billingAddressCursor.getColumnIndex(LocationSchema.STATE));
        if (tmp != null) state = tmp;
        tmp = billingAddressCursor.getString(billingAddressCursor.getColumnIndex(LocationSchema.ZIP));
        if (tmp != null) zip = tmp;

    }
    ((TextView)convertView.findViewById(R.id.cli_street)).setText(street);
    ((TextView)convertView.findViewById(R.id.cli_city_state_zip)).setText(city+", "+state+" "+zip);

Having said that, this is not the right way to fill this list. One DB query per list item is going to produce a performance dog. The simplest fix is to use a single INNER JOIN to retrieve all the information from both tables in one query. It's still possible the query will take too long if run on the UI thread. The preferred fix is to use the background Loader capability Android to do it without tying up the UI thread.

Upvotes: 3

stuckless
stuckless

Reputation: 6545

From looking at the code (which was a little confusing at first), it seems that you have a Custom CursorAdapter for customer and inside the bindView() of that adapter, you are doing a secondary query for the location of that person (ie, a Join from customer to location). This is going to perform badly, given that on the UI thread, for each customer, you are doing a secondary query.

Could this be solved more easily using the SimpleCursorAdapter and a join query between customer and location. (It seems to me the only purpose of the custom adapter is do "joining" of the customer and location, in a very inefficient way)

This way, the query would happen in the backgound, using Loaders perhaps, and the binding would be fairly efficient, never needing to do an alternate query.

The initial query may be something like..

select customer.name, location.address1, location.address2 from customer, location where customer.location_id = location._id

(I'm not sure what your field names are but, hopefully that conveys the basic structure)

Upvotes: 5

Related Questions