I have a RecyclerView
that binds a grocery item. Adding the item works perfectly. However, when I try to delete the item the app crashes and I get the IndexOutOfBoundsException
error.
The problem I am facing is in my onBindViewHolder()
. I tried to get the adapter position and that did not solve the problem. I also just tried using notifyDataSetChanged()
and that just deleted my entire list.
My question is what am I doing wrong and how do I fix this problem?
Here is the Adapter class:
private class GroceryAdapter extends RecyclerView.Adapter<GroceryHolder>{
private List<Grocery> groceries;
private GroceryHolder holder;
public GroceryAdapter(){
setGroceries(GroceryList.get(GroceryActivity.this).getGroceries());
}
public void setGroceries(List<Grocery> groceries){
this.groceries = groceries;
}
@Override
public GroceryHolder onCreateViewHolder(ViewGroup parent, int viewType){
LayoutInflater layoutInflater= LayoutInflater.from(GroceryActivity.this);
View view=layoutInflater.inflate(R.layout.grocery_list_row, parent, false);
holder =new GroceryHolder(view);
return holder;
}
@Override
public void onBindViewHolder(final GroceryHolder holder, final int position){
final Grocery grocery = groceries.get(position);
holder.bindGrocery(grocery);
//private Grocery grocery;
holder.deleteImageButton.setOnClickListener(new View.OnClickListener() {
@Override
public void onClick(View v) {
GroceryList groceryList = GroceryList.get(getApplicationContext());
groceryList.deleteGrocery(grocery);
groceryAdapter.setGroceries(GroceryList.get(getApplicationContext()).getGroceries());
//holder.getAdapterPosition();
groceryAdapter.notifyItemRemoved(position);
groceryAdapter.notifyItemRangeRemoved(position, groceries.size());
Toast.makeText(getApplicationContext(),"Item deleted", Toast.LENGTH_SHORT).show();
//finish();
}
});
}
@Override
public int getItemCount(){
return groceries.size();
}
}
}
Here is the logcat
E/AndroidRuntime: FATAL EXCEPTION: main
java.lang.IndexOutOfBoundsException: Inconsistency detected. Invalid view holder adapter positionViewHolder{426a0900 position=0 id=-1, oldPos=1, pLpos:1 scrap [attachedScrap] tmpDetached no parent}
at android.support.v7.widget.RecyclerView$Recycler.validateViewHolderForOffsetPosition(RecyclerView.java:5297)
at android.support.v7.widget.RecyclerView$Recycler.tryGetViewHolderForPositionByDeadline(RecyclerView.java:5479)
at android.support.v7.widget.RecyclerView$Recycler.getViewForPosition(RecyclerView.java:5440)
at android.support.v7.widget.RecyclerView$Recycler.getViewForPosition(RecyclerView.java:5436)
at android.support.v7.widget.LinearLayoutManager$LayoutState.next(LinearLayoutManager.java:2224)
at android.support.v7.widget.LinearLayoutManager.layoutChunk(LinearLayoutManager.java:1551)
at android.support.v7.widget.LinearLayoutManager.fill(LinearLayoutManager.java:1511)
at android.support.v7.widget.LinearLayoutManager.onLayoutChildren(LinearLayoutManager.java:595)
at android.support.v7.widget.RecyclerView.dispatchLayoutStep1(RecyclerView.java:3534)
at android.support.v7.widget.RecyclerView.dispatchLayout(RecyclerView.java:3310)
at android.support.v7.widget.RecyclerView.onLayout(RecyclerView.java:3844)
at android.view.View.layout(View.java:15745)
at android.view.ViewGroup.layout(ViewGroup.java:4867)
at android.widget.LinearLayout.setChildFrame(LinearLayout.java:1677)
at android.widget.LinearLayout.layoutVertical(LinearLayout.java:1531)
at android.widget.LinearLayout.onLayout(LinearLayout.java:1440)
at android.view.View.layout(View.java:15745)
at android.view.ViewGroup.layout(ViewGroup.java:4867)
at android.widget.FrameLayout.layoutChildren(FrameLayout.java:453)
at android.widget.FrameLayout.onLayout(FrameLayout.java:388)
at android.view.View.layout(View.java:15745)
at android.view.ViewGroup.layout(ViewGroup.java:4867)
at android.support.v7.widget.ActionBarOverlayLayout.onLayout(ActionBarOverlayLayout.java:437)
at android.view.View.layout(View.java:15745)
at android.view.ViewGroup.layout(ViewGroup.java:4867)
at android.widget.FrameLayout.layoutChildren(FrameLayout.java:453)
at android.widget.FrameLayout.onLayout(FrameLayout.java:388)
at android.view.View.layout(View.java:15745)
at android.view.ViewGroup.layout(ViewGroup.java:4867)
at android.widget.LinearLayout.setChildFrame(LinearLayout.java:1677)
at android.widget.LinearLayout.layoutVertical(LinearLayout.java:1531)
at android.widget.LinearLayout.onLayout(LinearLayout.java:1440)
at android.view.View.layout(View.java:15745)
at android.view.ViewGroup.layout(ViewGroup.java:4867)
at android.widget.FrameLayout.layoutChildren(FrameLayout.java:453)
at android.widget.FrameLayout.onLayout(FrameLayout.java:388)
at android.view.View.layout(View.java:15745)
at android.view.ViewGroup.layout(ViewGroup.java:4867)
at android.view.ViewRootImpl.performLayout(ViewRootImpl.java:2356)
at android.view.ViewRootImpl.performTraversals(ViewRootImpl.java:2069)
at android.view.ViewRootImpl.doTraversal(ViewRootImpl.java:1254)
at android.view.ViewRootImpl$TraversalRunnable.run(ViewRootImpl.java:6624)
at android.view.Choreographer$CallbackRecord.run(Choreographer.java:812)
at android.view.Choreographer.doCallbacks(Choreographer.java:612)
at android.view.Choreographer.doFrame(Choreographer.java:582)
at android.view.Choreographer$FrameDisplayEventReceiver.run(Choreographer.java:798)
at android.os.Handler.handleCallback(Handler.java:733)
at android.os.Handler.dispatchMessage(Handler.java:95)
at android.os.Looper.loop(Looper.java:146)
at android.app.ActivityThread.main(ActivityThread.java:5602)
at java.lang.reflect.Method.invokeNative(Native Method)
at java.lang.reflect.Method.invoke(Method.java:515)
at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:1283)
at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:1099)
at dalvik.system.NativeStart.main(Native Method)
You should not pass final int position
to your anonymous new View.OnClickListener()
Instead, use holder.getAdapterPosition()
:
private class GroceryAdapter extends RecyclerView.Adapter<GroceryHolder> {
private List<Grocery> groceries;
public GroceryAdapter(List<Grocery> groceries) {
this.groceries = groceries;
}
@Override
public GroceryHolder onCreateViewHolder(ViewGroup parent, int viewType){
LayoutInflater layoutInflater= LayoutInflater.from(GroceryActivity.this);
View view=layoutInflater.inflate(R.layout.grocery_list_row, parent, false);
holder = new GroceryHolder(view);
return holder;
}
@Override
public void onBindViewHolder(final GroceryHolder holder, int position){
int safePosition = holder.getAdapterPosition();
final Grocery grocery = groceries.get(safePosition);
holder.bindGrocery(grocery);
holder.deleteImageButton.setOnClickListener(new View.OnClickListener() {
@Override
public void onClick(View v) {
// delete item from the list
groceries.remove(safePosition);
groceryAdapter.notifyItemRemoved(safePosition);
Toast.makeText(getApplicationContext(),"Item deleted", Toast.LENGTH_SHORT).show();
}
});
}
@Override
public int getItemCount(){
return groceries.size();
}
}
In addition, there are few more unrelated issues:
it would be better to set setOnClickListener
not in onBindViewHolder
, but in onCreateViewHolder
Code around setting the list of groceries in constructor looks too complex. Could be much simpler to pass this list as an argument from your Activity/Fragment.
private GroceryHolder holder
as instance variable is unnecessary
Here is more context on holder.getAdapterPosition()
https://youtu.be/imsr8NrIAMs?t=2092