3

So I have a RecyclerView and in every xml that i pull into it there is a checkbox that when pressed removes that specific item from the RecyclerView. That's what I was trying to do at least. And as I understand for it to work i need to use notifyItemChanged(position); (it's right at the bottom).

However when I use notifyItemChanged(position); it only deletes the correct item if I press the checkboxes in these orders: 1,2,3,4,5 or 5,4,3,2,1. If I try to delete the 1st item, it deletes it but then if I want to delete the 3rd one, it deletes the 4th.

Another option I found was using notifyDataSetChanged(); and it works in any order however then no animations are shown.

And I'd like to have animations when items are added and deleted, so if there's a way of doing that with notifyDataSetChanged(); I would like help with that.

public class ListAdapter extends RecyclerView.Adapter{

SharedPreferences sharedpreferences, prefs;
public static final String MyPREFERENCES = "MyPrefs";
public static final String date = "date";
@Override
public RecyclerView.ViewHolder onCreateViewHolder(ViewGroup parent, int viewType) {
    View view = LayoutInflater.from(parent.getContext()).inflate(R.layout.nd_table, parent, false);
    return new ListViewHolder(view);
}

@Override
public void onBindViewHolder(RecyclerView.ViewHolder holder, final int adapterPosition) {
    //((ListViewHolder) holder).bindView(position);
    String data = hwName.get(adapterPosition);
    String data2 = hwDesc.get(adapterPosition);
    ((ListViewHolder) holder).mItemText.setText(data);
    ((ListViewHolder) holder).mItemText2.setText(data2);
    ((ListViewHolder) holder).buttonDelete.setOnClickListener(new View.OnClickListener(){
        @Override
        public void onClick(View view){
            removeItem(adapterPosition);
            notifyItemChanged(adapterPosition);
        }
    });
}


@Override
public int getItemCount() {
    return hwName.size();
}

public class ListViewHolder extends RecyclerView.ViewHolder implements View.OnClickListener {

    private TextView mItemText, mItemText2;
    public CheckBox buttonDelete;

    public ListViewHolder(View itemView) {
        super(itemView);
        mItemText = (TextView) itemView.findViewById(R.id.pam_name);
        mItemText2 = (TextView) itemView.findViewById(R.id.pam_nd);
        buttonDelete = (CheckBox) itemView.findViewById(R.id.nd_checkbox);
        itemView.setOnClickListener(this);
    }

    public void bindView(int position) {
        String value = hwName.get(position);
        String value2 = hwDesc.get(position);

        mItemText.setText(value);
        mItemText2.setText(value2);

        //notifyItemInserted(hwName.size());
    }


    public void onClick(View view) {

    }
}

public void removeItem(int position){
    hwName.remove(position);
    hwDesc.remove(position);
    //notifyDataSetChanged();
    notifyItemRemoved(position);
    notifyItemChanged(position);
}}

Crash report:

FATAL EXCEPTION: main Process: com.sajev.slush, PID: 14166 java.lang.IndexOutOfBoundsException: Index: 5, Size: 5 at java.util.ArrayList.remove(ArrayList.java:477) at com.sajev.slush.ListAdapter.removeItem(ListAdapter.java:89) at com.sajev.slush.ListAdapter$1.onClick(ListAdapter.java:45) at android.view.View.performClick(View.java:5646) at android.widget.CompoundButton.performClick(CompoundButton.java:123) at android.view.View$PerformClick.run(View.java:22459) at android.os.Handler.handleCallback(Handler.java:761) at android.os.Handler.dispatchMessage(Handler.java:98) at android.os.Looper.loop(Looper.java:156) at android.app.ActivityThread.main(ActivityThread.java:6523) at java.lang.reflect.Method.invoke(Native Method) at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:941) at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:831)

Edric
  • 24,639
  • 13
  • 81
  • 91
Sajev
  • 45
  • 2
  • 9
  • Post the crash logs – ADM Feb 27 '18 at 17:28
  • @ADM alright done – Sajev Feb 27 '18 at 17:32
  • **java.lang.IndexOutOfBoundsException** . Debug your code . – ADM Feb 27 '18 at 17:33
  • @ADM So i kind of understand why it crashes, let's say I delete the 1st item and the 2nd one also updates and there's no issue there, but if I press to delete the 3rd one now, it deletes the 4th one. But if the 4th one has already been moved to the 3rd position hwName.remove(position); and hwDesc.remove(position); has nothing to do delete from and it crashes. I knew that before but i just don't know how to update everything like notifyDataSetChanged(); does – Sajev Feb 27 '18 at 17:55

1 Answers1

4

There are two problems:

Don't call notifyItemChanged() after calling notifyItemRemoved()

Calling notifyItemRemoved() will tell the adapter that the item previously at position doesn't exist anymore, and that's all you need to do. Calling notifyItemChanged() tells the adapter that the item at position has been modified (perhaps its name changed, but nothing else), so it's not meant for use when you delete an item.

You're getting a crash because you call both of these in order. Imagine that you only have one item in your list, and then delete it. When you call notifyItemRemoved(), the adapter now knows that your list has zero items in it. But then you call notifyItemChanged(), and so the adapter tries to go get the first item... but the list is empty so you crash.

The position argument passed to onBindViewHolder() can not be final

(See https://www.youtube.com/watch?v=LqBlYJTfLP4 at ~43:10 for more info.)

The compiler can't stop you from adding the final keyword to the position argument of onBindViewHolder(), but it is a logical error to do so. When you call methods like notifyItemRemoved() or notifyItemInserted(), other view holders are not re-bound, and so the position argument will no longer reflect reality.

Do not use that position argument inside click listeners. Instead, look it up at runtime. Here's some updated code considering both problems:

@Override
public void onBindViewHolder(final RecyclerView.ViewHolder holder, int position) {
    ...
    ((ListViewHolder) holder).buttonDelete.setOnClickListener(new View.OnClickListener(){
        @Override
        public void onClick(View view){
            removeItem(holder.getAdapterPosition());
        }
    });
}

public void removeItem(int position){
    hwName.remove(position);
    hwDesc.remove(position);
    notifyItemRemoved(position);
}
Ben P.
  • 52,661
  • 6
  • 95
  • 123
  • You see the problem is still there. Imagine there are 3 items, on first click I can remove any with no problem, so let's say that I remove the very first. Now i want to remove the second item which has now been moved in place of the first but it is still being regarded as the second one and it calls the position to remove the second one, but because it's been moved into the position of the first it actually removes the third one, because the third one has been moved into the position of the second. – Sajev Feb 27 '18 at 18:23
  • @Sajev I updated my answer to cover why you should not use `final int position` in view holder event listeners. – Ben P. Feb 27 '18 at 18:33
  • Oh dude thanks a lot it works now, also maybe you have any idea about this little bug. If I check let's say on the first item then 5 and sometimes 6 items down another item get's his checkbox ticked. It doesn't actually activate the removal, just visual. I'm not quite sure what causes this. – Sajev Feb 27 '18 at 18:44
  • @Sajev my best guess is (1) user clicks on checkbox (2) item is deleted (3) old view holder is recycled (4) recycled view holder is used to display a different item (5) since the checkbox state is never un-set in `onBindViewHolder()`, it retains its checked state from #1 – Ben P. Feb 27 '18 at 18:46
  • You were right, okay great i fixed it now. Thanks again :) – Sajev Feb 27 '18 at 18:57