Question

I have never been happy with the code on my custom CursorAdapter until today I decided to review it and fix a little problem that was bothering me for a long time (interestingly enough, none of the users of my app ever reported such a problem).

Here's a small description of my question:

My custom CursorAdapter overrides newView() and bindView() instead of getView() as most examples I see. I use the ViewHolder pattern between these 2 methods. But my main issue was with the custom layout I'm using for each list item, it contains a ToggleButton.

The problem was that the button state was not kept when a list item view scrolled out of view and then scrollbed back into view. This problem existed because the cursor was never aware that the database data changed when the ToggleButton was pressed and it was always pulling the same data. I tried to requery the cursor when clicking the ToggleButton and that solved the problem, but it was very slow.

I have solved this issue and I'm posting the whole class here for review. I've commented the code thoroughly for this specific question to better explain my coding decisions.

Does this code look good to you? Would you improve/optimize or change it somehow?

P.S: I know the CursorLoader is an obvious improvement but I don't have time to deal with such big code rewrites for the time being. It's something I have in the roadmap though.

Here's the code:

public class NotesListAdapter extends CursorAdapter implements OnClickListener {

    private static class ViewHolder {
        ImageView icon;
        TextView title;
        TextView description;
        ToggleButton visibility;
    }

    private static class NoteData {
        long id;
        int iconId;
        String title;
        String description;
        int position;
    }

    private LayoutInflater mInflater;

    private NotificationHelper mNotificationHelper;
    private AgendaNotesAdapter mAgendaAdapter;

    /*
     * This is used to store the state of the toggle buttons for each item in the list
     */
    private List<Boolean> mToggleState;

    private int mColumnRowId;
    private int mColumnTitle;
    private int mColumnDescription;
    private int mColumnIconName;
    private int mColumnVisibility;

    public NotesListAdapter(Context context, Cursor cursor, NotificationHelper helper, AgendaNotesAdapter adapter) {
        super(context, cursor);

        mInflater = LayoutInflater.from(context);

        /*
         * Helper class to post notifications to the status bar and database adapter class to update
         * the database data when the user presses the toggle button in any of items in the list
         */
        mNotificationHelper = helper;
        mAgendaAdapter = adapter;

        /*
         * There's no need to keep getting the column indexes every time in bindView() (as I see in
         * a few examples) so I do it once and save the indexes in instance variables
         */
        findColumnIndexes(cursor);

        /*
         * Populate the toggle button states for each item in the list with the corresponding value
         * from each record in the database, but isn't this a slow operation?
         */
        for(mToggleState = new ArrayList<Boolean>(); !cursor.isAfterLast(); cursor.moveToNext()) {
            mToggleState.add(cursor.getInt(mColumnVisibility) != 0);
        }
    }

    @Override
    public View newView(Context context, Cursor cursor, ViewGroup parent) {
        View view = mInflater.inflate(R.layout.list_item_note, null);

        /*
         * The ViewHolder pattern is here only used to prevent calling findViewById() all the time
         * in bindView(), we only need to find all the views once
         */
        ViewHolder viewHolder = new ViewHolder();

        viewHolder.icon = (ImageView)view.findViewById(R.id.imageview_icon);
        viewHolder.title = (TextView)view.findViewById(R.id.textview_title);
        viewHolder.description = (TextView)view.findViewById(R.id.textview_description);
        viewHolder.visibility = (ToggleButton)view.findViewById(R.id.togglebutton_visibility);

        /*
         * I also use newView() to set the toggle button click listener for each item in the list
         */
        viewHolder.visibility.setOnClickListener(this);

        view.setTag(viewHolder);

        return view;
    }

    @Override
    public void bindView(View view, Context context, Cursor cursor) {
        Resources resources = context.getResources();

        int iconId = resources.getIdentifier(cursor.getString(mColumnIconName),
                "drawable", context.getPackageName());

        String title = cursor.getString(mColumnTitle);
        String description = cursor.getString(mColumnDescription);

        /*
         * This is similar to the ViewHolder pattern and it's need to access the note data when the
         * onClick() method is fired
         */
        NoteData noteData = new NoteData();

        /*
         * This data is needed to post a notification when the onClick() method is fired
         */
        noteData.id = cursor.getLong(mColumnRowId);
        noteData.iconId = iconId;
        noteData.title = title;
        noteData.description = description;

        /*
         * This data is needed to update mToggleState[POS] when the onClick() method is fired
         */
        noteData.position = cursor.getPosition();

        /*
         * Get our ViewHolder with all the view IDs found in newView()
         */
        ViewHolder viewHolder = (ViewHolder)view.getTag();

        /*
         * The Html.fromHtml is needed but the code relevant to that was stripped
         */
        viewHolder.icon.setImageResource(iconId);
        viewHolder.title.setText(Html.fromHtml(title));
        viewHolder.description.setText(Html.fromHtml(description));

        /*
         * Set the toggle button state for this list item from the value in mToggleState[POS]
         * instead of getting it from the database with 'cursor.getInt(mColumnVisibility) != 0'
         * otherwise the state will be incorrect if it was changed between the item view scrolling
         * out of view and scrolling back into view
         */
        viewHolder.visibility.setChecked(mToggleState.get(noteData.position));

        /*
         * Again, save the note data to be accessed when onClick() gets fired
         */
        viewHolder.visibility.setTag(noteData);
    }

    @Override
    public void onClick(View view) {
        /*
         * Get the new state directly from the toggle button state 
         */
        boolean visibility = ((ToggleButton)view).isChecked();

        /*
         * Get all our note data needed to post (or remove) a notification 
         */
        NoteData noteData = (NoteData)view.getTag();

        /*
         * The toggle button state changed, update mToggleState[POS] to reflect that new change
         */
        mToggleState.set(noteData.position, visibility);

        /*
         * Post the notification or remove it from the status bar depending on toggle button state
         */
        if(visibility) {
            mNotificationHelper.postNotification(
                    noteData.id, noteData.iconId, noteData.title, noteData.description);
        } else {
            mNotificationHelper.cancelNotification(noteData.id);
        }

        /*
         * Update the database note item with the new toggle button state, without the need to
         * requery the cursor (which is slow, I've tested it) to reflect the new toggle button state
         * in the list because the value was saved in mToggleState[POS] a few lines above
         */
        mAgendaAdapter.updateNote(noteData.id, null, null, null, null, visibility);
    }

    private void findColumnIndexes(Cursor cursor) {
        mColumnRowId = cursor.getColumnIndex(AgendaNotesAdapter.KEY_ROW_ID);
        mColumnTitle = cursor.getColumnIndex(AgendaNotesAdapter.KEY_TITLE);
        mColumnDescription = cursor.getColumnIndex(AgendaNotesAdapter.KEY_DESCRIPTION);
        mColumnIconName = cursor.getColumnIndex(AgendaNotesAdapter.KEY_ICON_NAME);
        mColumnVisibility = cursor.getColumnIndex(AgendaNotesAdapter.KEY_VISIBILITY);
    }

}

No correct solution

Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top