Rookie
Rookie

Reputation: 334

Why recyclerView always deletes last item

Here I am adding and deleting items in the editText in/from recyclerview. Adding is working fine but when I delete any item from the arraylist correct item is deleted but recyclerview is always deleting the last item. Here is the code

public static class ViewHolder extends RecyclerView.ViewHolder {
        EditText listItemEditText;
        TextView addNew;
        ImageView removeItem;
        public ViewHolder(View view) {
            super(view);
            listItemEditText = (EditText)view.findViewById(R.id.list_item);
            addNew = (TextView)view.findViewById(R.id.add);
            removeItem = (ImageView)view.findViewById(R.id.deleteItem);
        }
    }

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

    @Override
    public void onBindViewHolder(final ViewHolder holder, final int position) {
        holder.listItemEditText.setId(position);
        final TextWatcher textWatcher = new TextWatcher() {

            @Override
            public void onTextChanged(CharSequence s, int start, int before, int count) {


                try{
                    listItems.set(position,s.toString());
                }catch(Exception e){
                    listItems.add(position,s.toString());
                }
                if(s.length() >0 && !edited.get(position)){
                    edited.set(position, true);
                    holder.addNew.setVisibility(View.VISIBLE);
                }else if(s.length()==0){
                    holder.addNew.setVisibility(View.GONE);
                }
            }

            @Override
            public void beforeTextChanged(CharSequence s, int start, int count,
                    int after) {
                // TODO Auto-generated method stub

            }

            @Override
            public void afterTextChanged(Editable s) {
                // TODO Auto-generated method stub

            }
        };
        holder.listItemEditText.addTextChangedListener(textWatcher);
        holder.addNew.setOnClickListener(new OnClickListener() {

            @Override
            public void onClick(View v) {
                edited.add(position,true);
                edited.add(position+1,false);
                addEmptyRow(position+1);
                v.setVisibility(View.GONE);
            }
        });
        holder.removeItem.setOnClickListener(new OnClickListener() {

            @Override
            public void onClick(View v) {
                deleteRow(position,holder);
            }
        });
    }
    protected void addEmptyRow(int i) {
        listItems.add(i,"");
        notifyItemInserted(i);
    }

    @Override
    public NoteListItemsAdapter.ViewHolder onCreateViewHolder(ViewGroup parent, int arg1) {
        View v = LayoutInflater.from(MainActivity.activity).inflate(R.layout.list_notes, parent,false);
        ViewHolder holder = new ViewHolder(v);
        return holder;
    }

    private void deleteRow(int position, ViewHolder holder){
        Toast.makeText(MainActivity.activity, "position="+position+", Size="+listItems.size(), Toast.LENGTH_SHORT).show();
        listItems.remove(position);
        for(String ass:listItems){
            System.out.println(ass);
        }
        edited.remove(position);
        notifyItemRemoved(position);
        notifyDataSetChanged();

    }

Upvotes: 1

Views: 1331

Answers (1)

yigit
yigit

Reputation: 38253

This is wrong:

public void onBindViewHolder(final ViewHolder holder, final int position)

Should be:

public void onBindViewHolder(final ViewHolder holder, int position)

In other words, you cannot make the position final because as you add items, positions will change but RecyclerView will not rebind existing items just because their position changed (thus final position will become obselete).

Instead, use viewHolder.getAdapterPosition() inside on click to get up to date position.

Also, by doing this in onBind, you are creating unnecessary listener objects which is bad for memory utilization (more GC).

Implement it like this instead:

@Override
public NoteListItemsAdapter.ViewHolder onCreateViewHolder(ViewGroup parent, int arg1) {
    View v = LayoutInflater.from(MainActivity.activity).inflate(R.layout.list_notes, parent,false);
    final ViewHolder holder = new ViewHolder(v);
    holder.removeItem.setOnClickListener(new OnClickListener() {
        @Override
        public void onClick(View v) {
            int pos = holder.getAdapterPosition();
            if (pos != RecyclerView.NO_POSITION) {
                deleteRow(position,holder);
            }
        }
    });
    // do the same for all listeners
    return holder;
}

This will work well and you'll avoid churning objects.

Upvotes: 2

Related Questions