DiffUtil.ItemCallback sometimes loses oldItem and causes areContentsTheSame() to incorrectly return true

Question

I'm having an issue with the DiffUtil.ItemCallback library where the old state of a list item somehow ends up being lost and causes oldTask to have the same values as newTask.

This is happening when I check/uncheck a checkbox in my ListAdapter and onChanged() in my LiveData observer calls the DiffUtil.ItemCallback functions. As you can see inside onBindViewHolder(), that action calls setCompleted() which negates the value of isCompleted(). The expected behavior for an overdue task (in red) is to turn white whenever it's checkbox is checked. This is because isOverdue() can only return false when isCompleted() returns true.

onBindViewHolder():

@Override
public void onBindViewHolder(@NonNull ViewHolder viewHolder, int i) {
    SubTask subTask = getItem(i);
    Log.d(TAG, "onBindViewHolder: CALLED");
    viewHolder.subTaskName.setText(subTask.getName());

    Calendar dueDate = subTask.getDueDate();
    viewHolder.dueDate.setText(context.getResources().getString(R.string.due_date, dueDate.getDisplayName(Calendar.DAY_OF_WEEK, Calendar.LONG, Locale.getDefault()), dueDate.getDisplayName(Calendar.MONTH, Calendar.LONG, Locale.getDefault()), dueDate.get(Calendar.DAY_OF_MONTH), dueDate.get(Calendar.YEAR)));

    viewHolder.checkBox.setOnCheckedChangeListener(null);
    //check of sub task if completed
    if (subTask.isCompleted()) {
        viewHolder.checkBox.setChecked(true);
    } else {
        viewHolder.checkBox.setChecked(false);
    }

    if (subTask.isOverdue()) {
        viewHolder.card.setCardBackgroundColor(context.getColor(R.color.red));
        viewHolder.subTaskName.setTextColor(context.getColor(R.color.colorAccent));
        viewHolder.dueDate.setTextColor(context.getColor(R.color.colorAccent));
    } else {
        viewHolder.card.setCardBackgroundColor(context.getColor(R.color.colorAccent));
        viewHolder.subTaskName.setTextAppearance(R.style.TextAppearance_AppCompat_Large);
        viewHolder.dueDate.setTextColor(context.getColor(R.color.main_task_text_color));
    }

    viewHolder.checkBox.setOnCheckedChangeListener(new CompoundButton.OnCheckedChangeListener() {
        @Override
        public void onCheckedChanged(CompoundButton buttonView, boolean isChecked) {
            Log.d(TAG, "onCheckedChanged: called for subTask " + subTask.getName() + " isChecked = " + isChecked);
            viewModel = ViewModelProviders.of((FragmentActivity) context).get(ViewSubTasksForMainTaskViewModel.class);
            if (isChecked) {
                //if overdue, change card to white
                subTask.setCompleted(true);
                viewModel.updateSubTask(subTask);
            } else {
                //if overdue, set color back to red
                subTask.setCompleted(false);
                viewModel.updateSubTask(subTask);
            }
        }
    });

}

However, when areContentsTheSame() is trying to compare the values in the old list and the new list, it sometimes returns true when it shouldn't since isCompleted() is supposed to return different values for the old and new list items. It's seems like the old state of the list item somehow got lost. This causes the list items to be rendered incorrectly since onBindViewHolder() is not called. Here is a GIF that shows how the app is behaving. As you can see, list items are red even though they are checked and list items are white even though they are unchecked. You can also see that some list items render correctly but not all of them.

DiffCallback:

public class SubTaskDiffCallback {

private static final String TAG = "SubTaskDiffCallback";

static public DiffUtil.ItemCallback<SubTask> getSubTaskDiffCallback() {
    return new DiffUtil.ItemCallback<SubTask>() {
        @Override
        public boolean areItemsTheSame(@NonNull SubTask oldTask, @NonNull SubTask newTask) {
            Log.d(TAG, "areItemsTheSame: " + oldTask.getName() + " " + newTask.getName() + " " + Boolean.toString(oldTask.getId() == newTask.getId()));
            return oldTask.getId() == newTask.getId();
        }

        @Override
        public boolean areContentsTheSame(@NonNull SubTask oldTask, @NonNull SubTask newTask) {
            Log.d(TAG, "areContentsTheSame: oldTask " + oldTask.toString());
            Log.d(TAG, "areContentsTheSame: newTask " + newTask.toString());
            boolean contentsSame = oldTask.getName().equals(newTask.getName()) &&
                    oldTask.getDueDate().equals(newTask.getDueDate()) &&
                    oldTask.isCompleted() == newTask.isCompleted() &&
                    oldTask.getMainTaskId() == (newTask.getMainTaskId());
            Log.d(TAG, "areContentsTheSame = " + contentsSame);
            return contentsSame;
        }

        @Nullable
        @Override
        public Object getChangePayload(@NonNull SubTask oldTask, @NonNull SubTask newTask) {
            if (oldTask.isCompleted() != newTask.isCompleted()) {
                Log.d(TAG, "getChangePayload = false");
                return Boolean.FALSE;
            } else {
                return null;
            }
        }
    };
}
}

Here's a partial log for what happened in the GIF linked above:

2020-02-02 15:45:21.536 8226-8226/com.johnsorhannus.divideandconquer D/ViewSTForMTAdapter: onCheckedChanged: called for subTask Wash car isChecked = true
2020-02-02 15:45:21.555 8226-11254/com.johnsorhannus.divideandconquer D/SubTaskDiffCallback: areItemsTheSame: Wash car Wash car true
2020-02-02 15:45:21.555 8226-11254/com.johnsorhannus.divideandconquer D/SubTaskDiffCallback: areItemsTheSame: Read OS book Read OS book true
2020-02-02 15:45:21.556 8226-11254/com.johnsorhannus.divideandconquer D/SubTaskDiffCallback: areItemsTheSame: Read database book Read database book true
2020-02-02 15:45:21.556 8226-11254/com.johnsorhannus.divideandconquer D/SubTaskDiffCallback: areItemsTheSame: Read database book Read database book true
2020-02-02 15:45:21.556 8226-11254/com.johnsorhannus.divideandconquer D/SubTaskDiffCallback: areItemsTheSame: Read OS book Read OS book true
2020-02-02 15:45:21.556 8226-11254/com.johnsorhannus.divideandconquer D/SubTaskDiffCallback: areItemsTheSame: Wash car Wash car true
2020-02-02 15:45:21.557 8226-11254/com.johnsorhannus.divideandconquer D/SubTaskDiffCallback: areContentsTheSame: oldTask SubTask[name = Wash car, dueDate = Mar 17, 2019, overdue = false, completed = true]
2020-02-02 15:45:21.557 8226-11254/com.johnsorhannus.divideandconquer D/SubTaskDiffCallback: areContentsTheSame: newTask SubTask[name = Wash car, dueDate = Mar 17, 2019, overdue = false, completed = true]
2020-02-02 15:45:21.557 8226-11254/com.johnsorhannus.divideandconquer D/SubTaskDiffCallback: areContentsTheSame = true
2020-02-02 15:45:21.557 8226-11254/com.johnsorhannus.divideandconquer D/SubTaskDiffCallback: areContentsTheSame: oldTask SubTask[name = Read OS book, dueDate = Mar 20, 2019, overdue = true, completed = false]
2020-02-02 15:45:21.558 8226-11254/com.johnsorhannus.divideandconquer D/SubTaskDiffCallback: areContentsTheSame: newTask SubTask[name = Read OS book, dueDate = Mar 20, 2019, overdue = true, completed = false]
2020-02-02 15:45:21.558 8226-11254/com.johnsorhannus.divideandconquer D/SubTaskDiffCallback: areContentsTheSame = true
2020-02-02 15:45:21.558 8226-11254/com.johnsorhannus.divideandconquer D/SubTaskDiffCallback: areContentsTheSame: oldTask SubTask[name = Read database book, dueDate = Mar 23, 2019, overdue = true, completed = false]
2020-02-02 15:45:21.558 8226-11254/com.johnsorhannus.divideandconquer D/SubTaskDiffCallback: areContentsTheSame: newTask SubTask[name = Read database book, dueDate = Mar 23, 2019, overdue = true, completed = false]
2020-02-02 15:45:21.558 8226-11254/com.johnsorhannus.divideandconquer D/SubTaskDiffCallback: areContentsTheSame = true
2020-02-02 15:45:22.979 8226-8226/com.johnsorhannus.divideandconquer D/ViewSTForMTAdapter: onCheckedChanged: called for subTask Read OS book isChecked = true
2020-02-02 15:45:22.999 8226-11255/com.johnsorhannus.divideandconquer D/SubTaskDiffCallback: areItemsTheSame: Wash car Wash car true
2020-02-02 15:45:22.999 8226-11255/com.johnsorhannus.divideandconquer D/SubTaskDiffCallback: areItemsTheSame: Read OS book Read OS book true
2020-02-02 15:45:22.999 8226-11255/com.johnsorhannus.divideandconquer D/SubTaskDiffCallback: areItemsTheSame: Read database book Read database book true
2020-02-02 15:45:22.999 8226-11255/com.johnsorhannus.divideandconquer D/SubTaskDiffCallback: areItemsTheSame: Read database book Read database book true
2020-02-02 15:45:22.999 8226-11255/com.johnsorhannus.divideandconquer D/SubTaskDiffCallback: areItemsTheSame: Read OS book Read OS book true
2020-02-02 15:45:22.999 8226-11255/com.johnsorhannus.divideandconquer D/SubTaskDiffCallback: areItemsTheSame: Wash car Wash car true
2020-02-02 15:45:23.000 8226-11255/com.johnsorhannus.divideandconquer D/SubTaskDiffCallback: areContentsTheSame: oldTask SubTask[name = Wash car, dueDate = Mar 17, 2019, overdue = false, completed = true]
2020-02-02 15:45:23.001 8226-11255/com.johnsorhannus.divideandconquer D/SubTaskDiffCallback: areContentsTheSame: newTask SubTask[name = Wash car, dueDate = Mar 17, 2019, overdue = false, completed = true]
2020-02-02 15:45:23.001 8226-11255/com.johnsorhannus.divideandconquer D/SubTaskDiffCallback: areContentsTheSame = true
2020-02-02 15:45:23.002 8226-11255/com.johnsorhannus.divideandconquer D/SubTaskDiffCallback: areContentsTheSame: oldTask SubTask[name = Read OS book, dueDate = Mar 20, 2019, overdue = true, completed = false]
2020-02-02 15:45:23.002 8226-11255/com.johnsorhannus.divideandconquer D/SubTaskDiffCallback: areContentsTheSame: newTask SubTask[name = Read OS book, dueDate = Mar 20, 2019, overdue = false, completed = true]
2020-02-02 15:45:23.002 8226-11255/com.johnsorhannus.divideandconquer D/SubTaskDiffCallback: areContentsTheSame = false
2020-02-02 15:45:23.003 8226-11255/com.johnsorhannus.divideandconquer D/SubTaskDiffCallback: areContentsTheSame: oldTask SubTask[name = Read database book, dueDate = Mar 23, 2019, overdue = true, completed = false]
2020-02-02 15:45:23.004 8226-11255/com.johnsorhannus.divideandconquer D/SubTaskDiffCallback: areContentsTheSame: newTask SubTask[name = Read database book, dueDate = Mar 23, 2019, overdue = true, completed = false]
2020-02-02 15:45:23.004 8226-11255/com.johnsorhannus.divideandconquer D/SubTaskDiffCallback: areContentsTheSame = true
2020-02-02 15:45:23.004 8226-8226/com.johnsorhannus.divideandconquer D/SubTaskDiffCallback: getChangePayload = false
2020-02-02 15:45:23.014 8226-8226/com.johnsorhannus.divideandconquer D/ViewSTForMTAdapter: onBindViewHolder: CALLED
2020-02-02 15:45:24.275 8226-8226/com.johnsorhannus.divideandconquer D/ViewSTForMTAdapter: onCheckedChanged: called for subTask Read database book isChecked = true
2020-02-02 15:45:24.294 8226-11254/com.johnsorhannus.divideandconquer D/SubTaskDiffCallback: areItemsTheSame: Wash car Wash car true
2020-02-02 15:45:24.294 8226-11254/com.johnsorhannus.divideandconquer D/SubTaskDiffCallback: areItemsTheSame: Read OS book Read OS book true
2020-02-02 15:45:24.294 8226-11254/com.johnsorhannus.divideandconquer D/SubTaskDiffCallback: areItemsTheSame: Read database book Read database book true
2020-02-02 15:45:24.294 8226-11254/com.johnsorhannus.divideandconquer D/SubTaskDiffCallback: areItemsTheSame: Read database book Read database book true
2020-02-02 15:45:24.294 8226-11254/com.johnsorhannus.divideandconquer D/SubTaskDiffCallback: areItemsTheSame: Read OS book Read OS book true
2020-02-02 15:45:24.294 8226-11254/com.johnsorhannus.divideandconquer D/SubTaskDiffCallback: areItemsTheSame: Wash car Wash car true
2020-02-02 15:45:24.295 8226-11254/com.johnsorhannus.divideandconquer D/SubTaskDiffCallback: areContentsTheSame: oldTask SubTask[name = Wash car, dueDate = Mar 17, 2019, overdue = false, completed = true]
2020-02-02 15:45:24.295 8226-11254/com.johnsorhannus.divideandconquer D/SubTaskDiffCallback: areContentsTheSame: newTask SubTask[name = Wash car, dueDate = Mar 17, 2019, overdue = false, completed = true]
2020-02-02 15:45:24.295 8226-11254/com.johnsorhannus.divideandconquer D/SubTaskDiffCallback: areContentsTheSame = true
2020-02-02 15:45:24.296 8226-11254/com.johnsorhannus.divideandconquer D/SubTaskDiffCallback: areContentsTheSame: oldTask SubTask[name = Read OS book, dueDate = Mar 20, 2019, overdue = false, completed = true]
2020-02-02 15:45:24.296 8226-11254/com.johnsorhannus.divideandconquer D/SubTaskDiffCallback: areContentsTheSame: newTask SubTask[name = Read OS book, dueDate = Mar 20, 2019, overdue = false, completed = true]
2020-02-02 15:45:24.296 8226-11254/com.johnsorhannus.divideandconquer D/SubTaskDiffCallback: areContentsTheSame = true
2020-02-02 15:45:24.296 8226-11254/com.johnsorhannus.divideandconquer D/SubTaskDiffCallback: areContentsTheSame: oldTask SubTask[name = Read database book, dueDate = Mar 23, 2019, overdue = true, completed = false]
2020-02-02 15:45:24.297 8226-11254/com.johnsorhannus.divideandconquer D/SubTaskDiffCallback: areContentsTheSame: newTask SubTask[name = Read database book, dueDate = Mar 23, 2019, overdue = false, completed = true]
2020-02-02 15:45:24.297 8226-11254/com.johnsorhannus.divideandconquer D/SubTaskDiffCallback: areContentsTheSame = false
2020-02-02 15:45:24.297 8226-8226/com.johnsorhannus.divideandconquer D/SubTaskDiffCallback: getChangePayload = false
2020-02-02 15:45:24.308 8226-8226/com.johnsorhannus.divideandconquer D/ViewSTForMTAdapter: onBindViewHolder: CALLED
2020-02-02 15:45:26.590 8226-8226/com.johnsorhannus.divideandconquer D/ViewSTForMTAdapter: onCheckedChanged: called for subTask Read database book isChecked = false
2020-02-02 15:45:26.608 8226-11255/com.johnsorhannus.divideandconquer D/SubTaskDiffCallback: areItemsTheSame: Wash car Wash car true
2020-02-02 15:45:26.608 8226-11255/com.johnsorhannus.divideandconquer D/SubTaskDiffCallback: areItemsTheSame: Read OS book Read OS book true
2020-02-02 15:45:26.608 8226-11255/com.johnsorhannus.divideandconquer D/SubTaskDiffCallback: areItemsTheSame: Read database book Read database book true
2020-02-02 15:45:26.608 8226-11255/com.johnsorhannus.divideandconquer D/SubTaskDiffCallback: areItemsTheSame: Read database book Read database book true
2020-02-02 15:45:26.608 8226-11255/com.johnsorhannus.divideandconquer D/SubTaskDiffCallback: areItemsTheSame: Read OS book Read OS book true
2020-02-02 15:45:26.608 8226-11255/com.johnsorhannus.divideandconquer D/SubTaskDiffCallback: areItemsTheSame: Wash car Wash car true
2020-02-02 15:45:26.609 8226-11255/com.johnsorhannus.divideandconquer D/SubTaskDiffCallback: areContentsTheSame: oldTask SubTask[name = Wash car, dueDate = Mar 17, 2019, overdue = false, completed = true]
2020-02-02 15:45:26.609 8226-11255/com.johnsorhannus.divideandconquer D/SubTaskDiffCallback: areContentsTheSame: newTask SubTask[name = Wash car, dueDate = Mar 17, 2019, overdue = false, completed = true]
2020-02-02 15:45:26.609 8226-11255/com.johnsorhannus.divideandconquer D/SubTaskDiffCallback: areContentsTheSame = true
2020-02-02 15:45:26.610 8226-11255/com.johnsorhannus.divideandconquer D/SubTaskDiffCallback: areContentsTheSame: oldTask SubTask[name = Read OS book, dueDate = Mar 20, 2019, overdue = false, completed = true]
2020-02-02 15:45:26.610 8226-11255/com.johnsorhannus.divideandconquer D/SubTaskDiffCallback: areContentsTheSame: newTask SubTask[name = Read OS book, dueDate = Mar 20, 2019, overdue = false, completed = true]
2020-02-02 15:45:26.610 8226-11255/com.johnsorhannus.divideandconquer D/SubTaskDiffCallback: areContentsTheSame = true
2020-02-02 15:45:26.611 8226-11255/com.johnsorhannus.divideandconquer D/SubTaskDiffCallback: areContentsTheSame: oldTask SubTask[name = Read database book, dueDate = Mar 23, 2019, overdue = true, completed = false]
2020-02-02 15:45:26.611 8226-11255/com.johnsorhannus.divideandconquer D/SubTaskDiffCallback: areContentsTheSame: newTask SubTask[name = Read database book, dueDate = Mar 23, 2019, overdue = true, completed = false]
2020-02-02 15:45:26.611 8226-11255/com.johnsorhannus.divideandconquer D/SubTaskDiffCallback: areContentsTheSame = true

I know for sure that setCompleted() changed the value of isCompleted() because when you exit and reenter the activity, it renders properly.

Try to replace checked changed listener with onclick listener. Thing with checked changed listener is it triggers when you programatically call `setChecked`, and since you don't properly consume change payload you're leaking previous listener whenever viewholders are reused. — Pawel, Feb 02 '20 at 21:41
@Pawel I replaced the checked changed listener with [this](https://gist.github.com/JohnSorhannus/6d296f80bbc4fb55a607ad08976c1ef2) , but it did not solve the problem, unfortunately. — John S., Feb 02 '20 at 22:37

score 4 · Answer 1 · answered Feb 03 '20 at 19:11

4

DiffUtil stores the old items by reference - it doesn't copy them. Your SubTask class is mutable, so when you call setCompleted, you're changing the old one that DiffUtil has a reference to as well. DiffUtil is only able to compare to a list of old, unchanged objects. Because you changed the old object, and it only has a reference to the object that you've now changed, it doesn't know the correct old value.

answered Feb 03 '20 at 19:11

Ryan M

18,333
31
67
74

I see. The point of using `DiffUtil.ItemCallback` here was to provide for animations when a list item is removed by the user. What should I be changing about my implementation to make it behave the way I want? `SubTask` needs to be mutable since a change in its contents needs to be reflected in my Room database and eventually onto the screen by changing the color of its card based on the return value of `isOverdue()`. – John S. Feb 04 '20 at 00:13

DiffUtil.ItemCallback sometimes loses oldItem and causes areContentsTheSame() to incorrectly return true

1 Answers1