android delete any item from listview deleting last element

Question

I made a list view u can delete an item by touch the row of that listview, but it always deleting the last item, and after some delete times it shows index out of bound error

system was when u toch an item a dialogue box open u click print then item will removed the item and open an activity for few seconds.

here is the list adapter class

   @Override
    public View getView(final int position, View convertView, ViewGroup parent) {
        LayoutInflater inflater = (LayoutInflater) context
                .getSystemService(Context.LAYOUT_INFLATER_SERVICE);
        final int positionplayer = position;
        ViewHolderaway1 holder;

        if (convertView == null) {

            View row = inflater.inflate(R.layout.readonlyvendorlist, parent,
                    false);

            row.setOnClickListener(new OnClickListener() {

                @Override
                public void onClick(View v) {
                    // TODO Auto-generated method stub
                    // set title
                    try {
                        AlertDialog.Builder alertDialogBuilder = new AlertDialog.Builder(
                                context);

                        alertDialogBuilder.setTitle(values.get(positionplayer).Voucherref
                                + "  Sell");

                        // set dialog message
                        alertDialogBuilder
                                .setMessage(
                                        values.get(positionplayer)
                                                .getVoucherref() + "For Print")
                                .setCancelable(false)
                                .setPositiveButton("Print Voucher",
                                        new DialogInterface.OnClickListener() {
                                            public void onClick(
                                                    DialogInterface dialog,
                                                    int id) {
                                                // if this button is clicked,
                                                // close

                                                PrintTestAcitvity.printettext = values
                                                        .get(positionplayer).PrinterText;
                                                ListService.printerlist
                                                        .remove(positionplayer);

                                                Datasync.storedataprint(context);
                                                notifyDataSetChanged();
                                                Intent ia = new Intent(context,
                                                        PrintTestAcitvity.class);

                                                context.startActivity(ia);

                                            }
                                        })
                                .setNegativeButton("Cancell",
                                        new DialogInterface.OnClickListener() {
                                            public void onClick(
                                                    DialogInterface dialog,
                                                    int id) {
                                                // if this button is clicked,
                                                // just close
                                                // the dialog box and do nothing
                                                Toast.makeText(
                                                        context,
                                                        "Printing Data store for locally",
                                                        Toast.LENGTH_LONG)
                                                        .show();

                                                dialog.cancel();
                                            }
                                        });

                        // create alert dialog
                        AlertDialog alertDialog = alertDialogBuilder.create();

                        // show it
                        alertDialog.show();
                    } catch (IndexOutOfBoundsException e) {
                        // TODO: handle exception
                        notifyDataSetChanged();
                    }
                }
            });

            holder = new ViewHolderaway1();

            holder.ProductItemID = (TextView) row
                    .findViewById(R.id.ProductItemID);

            holder.VoucherCost = (TextView) row.findViewById(R.id.VoucherCost);

            holder.SupplierID = (TextView) row.findViewById(R.id.SupplierID);

            Log.d("goru", "gadha");

            row.setTag(holder);

            holder = (ViewHolderaway1) row.getTag();
//          Printer Productsdata = values.get(positionplayer);
//          if (Productsdata != null) {
//              holder.ProductItemID.setText("Print");
//              holder.VoucherCost.setText(Productsdata.getVoucherref());
//              // holder.SupplierID.setText(resid)
//              // holder.SupplierID.setVisibility(View.GONE);
//
//              if (Productsdata.getVoucherref().contains("Voda")) {
//                  holder.VoucherCost.setBackgroundColor(Color.RED);
//                  holder.VoucherCost.setTextColor(Color.WHITE);
//                  holder.SupplierID.setBackgroundDrawable(getContext()
//                          .getResources().getDrawable(R.drawable.voda));
//              }
//              if (Productsdata.getVoucherref().contains("Eco")) {
//                  holder.VoucherCost.setBackgroundColor(Color.BLUE);
//                  holder.VoucherCost.setTextColor(Color.WHITE);
//                  holder.SupplierID.setBackgroundDrawable(getContext()
//                          .getResources().getDrawable(R.drawable.eco));
//              }
//
//          }
            convertView = row;
        }else
        {
            holder = (ViewHolderaway1) convertView.getTag();
        }
            Printer Productsdata = values.get(positionplayer);
            if (Productsdata != null) {
                holder.ProductItemID.setText("Print");
                holder.VoucherCost.setText(Productsdata.getVoucherref());
                // holder.SupplierID.setText(resid)
                // holder.SupplierID.setVisibility(View.GONE);

                if (Productsdata.getVoucherref().contains("Voda")) {
                    holder.VoucherCost.setBackgroundColor(Color.RED);
                    holder.VoucherCost.setTextColor(Color.WHITE);
                    holder.SupplierID.setBackgroundDrawable(getContext()
                            .getResources().getDrawable(R.drawable.voda));
                }
                if (Productsdata.getVoucherref().contains("Eco")) {
                    holder.VoucherCost.setBackgroundColor(Color.BLUE);
                    holder.VoucherCost.setTextColor(Color.WHITE);
                    holder.SupplierID.setBackgroundDrawable(getContext()
                            .getResources().getDrawable(R.drawable.eco));
                }

            }
        return convertView;
    }

score 2 · Accepted Answer · edited May 23 '17 at 12:04

2

There are some major flaws with your logic in getView(), this is the biggest one:

    if (convertView != null)
    {  
        holder = new ViewHolderaway1();


        return convertView;
    }

This returns the recycled row without altering it... At a minimum you need to change the written content of the layout.

Look at the code in this answer: https://stackoverflow.com/a/4145996/1267661. You should use this as template. It demonstrates how to properly use a ViewHolder and display accurate data after Views are returned from ListView's RecycleBin.

Addition
This new code is much more efficient! Also I think I see why only the last row is deleted. In your AlertDialog you reference positionplayer but this onClick() code is run after getView() has finished, so positionplayer is not helpful here. Since your OnClickListener refers to the whole row, you should use the OnItemClickListener in your Activity instead and this will help delete the appropriate row:

listView.setOnItemClickListener(new OnItemClickListener() {
    @Override
    public void onItemClick(AdapterView<?> parent, View view, int position, long id) {
        // Move your AlertDialog code here and use position instead of positionplayer
    }
});

edited May 23 '17 at 12:04

Community

1
1

answered Jan 11 '13 at 18:52

Sam

86,580
20
181
179

1

They center on the ViewHolder, you don't actually use any of it's benefits. If you follow the template and only use `findViewById()` when `convertView == null`, you'll have faster code. A minor improvement is to only fetch the LayoutInflater once in your constructor, since it is always the same. – Sam Jan 11 '13 at 19:08
1

If you re-write your code, post it in your question and then notify when you've done this. I'll happily give you more pointers (I just won't re-write you code for you.) – Sam Jan 11 '13 at 19:10
1

This code is much more efficient! Is the correct row deleted now? – Sam Jan 11 '13 at 19:34
1

I didn't think so, but my new answer might. :) – Sam Jan 11 '13 at 19:41
@Sam can you please answer the similar question at : http://stackoverflow.com/questions/23489233/deleting-any-list-row-deletes-the-last-row-in-android-fragment?noredirect=1#comment36018731_23489233 – user2011302 May 06 '14 at 10:25

android delete any item from listview deleting last element

1 Answers1