1

I am trying to delete multiple items from recyclerView. The items in the recyclerView has an attribute isSelected. I am setting the attribute isSelected to true when user clicks on the recyclerView item. After selection user can click delete button present in the options menu to delete the selected items. The code has unexpected behaviour, like when delete button is pressed, some of the selected items are deleted while some are not. Also some of the items get automatically selected at random positions.

Model.java

public class Model {
    private String text;
    private boolean isSelected = false;

    public Model(String text) {
        this.text = text;
    }

    public String getText() {
        return text;
    }

    public void setSelected(boolean selected) {
        isSelected = selected;
    }


    public boolean isSelected() {
        return isSelected;
    }
}

RecyclerViewAdapter.java

public class RecyclerViewAdapter extends RecyclerView.Adapter<RecyclerViewAdapter.MyViewHolder> {

    private List<Model> mModelList;
    private Context mCtx;

    public RecyclerViewAdapter(Context ctx, List<Model> modelList) {
        this.mModelList = modelList;
        this.mCtx = ctx;
    }

    @Override
    public MyViewHolder onCreateViewHolder(ViewGroup parent, int viewType) {
        LayoutInflater inflater = LayoutInflater.from(mCtx);
        View view = inflater.inflate(R.layout.item_row, parent, false);
        return new MyViewHolder(view);
    }

    @Override
    public void onBindViewHolder(final MyViewHolder holder, final int position) {
        final Model model = mModelList.get(position);
        holder.view.setBackgroundColor(model.isSelected() ? Color.CYAN : Color.WHITE);
        holder.tvItems.setText(model.getText());
        holder.tvItems.setOnClickListener(new View.OnClickListener() {
            @Override
            public void onClick(View view) {
                model.setSelected(!model.isSelected());
                holder.tvItems.setBackgroundColor(model.isSelected() ? Color.CYAN : Color.WHITE);
                notifyItemChanged(position);
            }
        });
    }

    @Override
    public int getItemCount() {
        return mModelList == null ? 0 : mModelList.size();
    }

    public class MyViewHolder extends RecyclerView.ViewHolder {
        private TextView tvItems;
        private View view;

        public MyViewHolder(View itemView) {
            super(itemView);
            view = itemView;
            tvItems = itemView.findViewById(R.id.tvItems);
        }
    }
}

MainActivity.java

public class MainActivity extends AppCompatActivity {

    private List<Model> mModelList;
    private RecyclerView mRecyclerView;
    private RecyclerView.Adapter mAdapter;

    @Override
    protected void onCreate(Bundle savedInstanceState) {
        super.onCreate(savedInstanceState);
        setContentView(R.layout.activity_main);

        mRecyclerView = (RecyclerView) findViewById(R.id.rvListItems);
        mAdapter = new RecyclerViewAdapter(this, getListData());
        LinearLayoutManager manager = new LinearLayoutManager(MainActivity.this);
        mRecyclerView.setHasFixedSize(true);
        mRecyclerView.setLayoutManager(manager);
        mRecyclerView.setAdapter(mAdapter);
    }

    private List<Model> getListData() {
        mModelList = new ArrayList<>();
        for (int i = 1; i <= 25; i++) {
            mModelList.add(new Model("TextView " + i));
        }
        return mModelList;
    }


    private void deleteSelectedItems() {
        if (mModelList != null){
            for (int i=0; i<mModelList.size(); i++){
                if (mModelList.get(i).isSelected()){
                    Log.d("testingTAG", String.valueOf(i));
                    mModelList.remove(i);
                    mAdapter.notifyItemRemoved(i);
                    mAdapter.notifyItemRangeChanged(i, mModelList.size());
                    i--;
                }
            }
        }

    }


    @Override
    public boolean onCreateOptionsMenu(Menu menu) {
        // Inflate the menu; this adds items to the action bar if it is present.
        getMenuInflater().inflate(R.menu.menu_main, menu);
        return true;
    }

    @Override
    public boolean onOptionsItemSelected(MenuItem item) {
        switch (item.getItemId()){
            case R.id.action_delete:
                deleteSelectedItems();
                break;
        }
        return super.onOptionsItemSelected(item);
    }
}

Any kind of help will be greatly appreciated.

MONU KUMAR
  • 156
  • 3
  • 11

2 Answers2

1

You should use holder.getAdapterPosition() inside your clicklistener .Change your onclick as given below and try.

public void onClick(View view) {
                final Model model = mModelList.get(holder.getAdapterPosition());
                model.setSelected(!model.isSelected());
                holder.tvItems.setBackgroundColor(model.isSelected() ? Color.CYAN : Color.WHITE);
                notifyItemChanged(holder.getAdapterPosition());
            }
pop
  • 559
  • 3
  • 13
0

Answer from @pop selects items, change background color to CYAN if item is selected and inverse selected flag of Model class instance.

you should delete selected items from your mModelList using a iterator, you can't delete items from list inside for loop

Calling remove in foreach loop in Java

Thracian
  • 43,021
  • 16
  • 133
  • 222
  • @Thracian the logic used by him is correct, not optimized but correct. – nitinkumarp Jun 26 '18 at 05:44
  • @nitinkumarp No, it's not. Let say list has 3 items. Index starts at 0 and ends in 2. He lowers i-- but loop will run until i is 2 to but there won't be mModelList.get(2) since now list contains 2 elements. Correct way to remove from a list is to use iterator. https://stackoverflow.com/questions/10738634/delete-data-from-arraylist-with-a-for-loop – Thracian Jun 26 '18 at 06:41
  • @Thracian don't you think the condition `i – MONU KUMAR Jun 26 '18 at 08:26
  • @nitinkumarp why do you say it is not optimized? Could you please point out mistakes and if possible provide an efficient method. – MONU KUMAR Jun 26 '18 at 08:34
  • @MONUKUMAR To test, create a list of 3 items. If your final i value before i-- is 2 after removing an item from the list, app will crash because your list won't have item with index 2 mList.get(2) won't exist. Check out the links i posted. – Thracian Jun 26 '18 at 08:48
  • @Thracian, `i=2` implies the condition inside `if` was never true. Now when `i = 2` you say, the condition inside `if` becomes true and so on. I don't think that's a problem. item at index `2` will be removed and finally value `i` will be `1`. Also the app doesn't crash. I have tested it. – MONU KUMAR Jun 26 '18 at 09:26
  • To optimize your `for` use `else` statement and write `++i` inside else. Remove i++ from for statement `for (int i=0; i – nitinkumarp Jun 26 '18 at 09:48