Use of ViewHolder is giving unusual output

Question

I'm using custom list view in which each row contains two image view and a text view. I'm using view holder in getView() method of custom adapter. I've set Onclicklistener() on one of the image view in getview() method only also onSelectItemListener() in activity on listview. Now, When I click on imageview it effects particular row but when I scroll down I got random rows effected by that click. It is happening when I click On imageview But this doesn't happen when I don't use viewholder. I'm really not getting why this is happening. Please help me.

Here is my code of getView() method:

public View getView(int position, View convertView, ViewGroup parent) {
    final int myposition = position;
    // TODO Auto-generated method stub

    if (convertView == null) {
        LayoutInflater inflater = LayoutInflater.from(context);
        convertView = inflater.inflate(R.layout.fileviewrowfrnview_layout, null);
        ViewHolder holder = new ViewHolder();
        holder.icon = (ImageView) convertView.findViewById(R.id.icon2);
        holder.text = (TextView) convertView.findViewById(R.id.file_name2);
        holder.download = (ImageView) convertView.findViewById(R.id.play_buttonOropen2);
        convertView.setTag(holder);
    }
    final ViewHolder holder = (ViewHolder) convertView.getTag();


    holder.download.setOnClickListener(new View.OnClickListener() {

        @
        Override
        public void onClick(View v) {
            // TODO Auto-generated method stub  
            if (frnshare_list.get(myposition).getFileType().trim()
                .equalsIgnoreCase("file")) {
                // it is file
                if (frnshare_list.get(myposition).getDownloadStatus().trim()
                    .equalsIgnoreCase("0")) {
                    // is is not a downloaded file do download it
                    holder.icon.setVisibility(View.GONE);
                    holder.download.setVisibility(View.GONE);
                    holder.text.setTextSize(14);

                    Log.e("file to be download", "" + frnshare_list.get(myposition).getFileName());
                    if (frnshare_list.get(myposition).getFileName().length() > 15)
                        holder.text.setText(frnshare_list.get(myposition).getFileName().subSequence(0, 15) + " will be downloaded soon.");
                    else
                        holder.text.setText(frnshare_list.get(myposition).getFileName() + " will be downloaded soon.");
                    UserService us = new UserService(context);
                    Frnshared fdto = frnshare_list.get(myposition);
                    fdto.setDownloadStatus("2"); // in queue
                    fdto.setMessageId("0");
                    us.updateDowloadStatusById(fdto);
                    us.updatemsgIdbyId(fdto);
                    if (CommonUtility.isNetworkAvailable(context)) {
                        AsyncTask < Void, Void, Void > httpconnection = new AsyncTask < Void, Void, Void > () {@
                            Override
                            protected Void doInBackground(Void...params) {
                                // TODO Auto-generated method stub
                                NetworkCommunication nc = new NetworkCommunication(
                                    context);
                                try {
                                    UserService us = new UserService(
                                        context);
                                    Frnshared fdto = frnshare_list
                                        .get(myposition);
                                    Log.i("file to be download", "" + fdto.getFileName());
                                    fdto.setDownloadStatus("2"); // in queue
                                    fdto.setMessageId("0");
                                    us.updateDowloadStatusById(fdto);
                                    us.updatemsgIdbyId(fdto);
                                    String response = nc
                                        .MyHttpPostDownload(frnshare_list
                                        .get(myposition));
                                    ParsersAndDataInsertion.DownloadRequestResponseParser(
                                        response, context,
                                        frnshare_list.get(myposition));

                                } catch (IOException e) {
                                    // TODO Auto-generated catch block
                                    e.printStackTrace();
                                }
                                // http call for download
                                return null;
                            }
                        }.execute(null, null, null);
                    }
                    Apitable(adto);
                }

            } else if (frnshare_list.get(myposition)
                .getDownloadStatus().equalsIgnoreCase("1")) {

                Intent intent = new Intent();
                intent.setAction(android.content.Intent.ACTION_VIEW);
                File file = new File(frnshare_list.get(myposition)
                    .getFileLocation());
                intent.setDataAndType(Uri.fromFile(file), "audio/*");

                context.startActivity(intent);

            } else if (frnshare_list.get(myposition)
                .getDownloadStatus().equalsIgnoreCase("2")) {
                Toast.makeText(context, "already request snet", 2000)
                    .show();
            }
        } else if (frnshare_list.get(myposition).getFileType()
            .equalsIgnoreCase("folder")) {}

    }
    });

// view purpose


String file_name = frnshare_list.get(position).getFileName();
if (file_name.length() > 15)
    holder.text.setText(file_name.substring(0, 15) + "");
else
    holder.text.setText(file_name); // frn name

try {
    if (frnshare_list.get(myposition).getFileType()
        .equalsIgnoreCase("file")) { // it is file
        holder.icon.setImageResource(R.drawable.mp3_icon);
        if (frnshare_list.get(myposition).getDownloadStatus()
            .equalsIgnoreCase("0")) {
            Log.i("file is ", "" + frnshare_list.get(myposition).getFileName());
            holder.download.setImageResource(R.drawable.download_png);
        } else if (frnshare_list.get(myposition).getDownloadStatus()
            .equalsIgnoreCase("1")) {

        } else if (frnshare_list.get(myposition).getDownloadStatus()
            .equalsIgnoreCase("2")) {
            Log.i("file is in queue", "" + frnshare_list.get(myposition).getFileName());
            holder.icon.setVisibility(View.GONE);
            holder.download.setVisibility(View.GONE);
            holder.text.setTextSize(14);
            if (frnshare_list.get(myposition).getFileName().length() > 15)
                holder.text.setText(frnshare_list.get(myposition).getFileName().substring(0, 1);
            else
                holder.text.setText(frnshare_list.get(myposition).getFileName() );
        }
    } else if (frnshare_list.get(myposition).getFileType()
        .equalsIgnoreCase("folder")) {
        holder.icon.setImageResource(R.drawable.folder);

    }
} catch (Exception e) {
    // TODO: handle exception
    Intent i = new Intent(context, TabViewActivity.class);
    i.putExtra("show", "frntab");
    context.startActivity(i);

    BugSenseHandler.sendException(e);
    e.printStackTrace();
}


return convertView;
}

Answer 1

I think your code is the normal way of implementing it, however due to recycling your problem in being created.

What you can do is 1. check of the position that you are getting directly in the method - it shoould be in order of repaint 2. instead of reusing the already set content in your holder, once you get the holder from the convert view, set that content from other data source like a List<<>> - so you would save time in re-creating the view, while content will be in your other data holder.

Answer 2

You're doing async tasks off of a ListView. That's asking for trouble. Here's the thing- ListView recycles rows. So by the time the task finishes, it may actually be pointing at a totally different position than it is when it starts. You have to be very careful and make sure either the async task has no effect on the UI at all, that it forces the entire list to redraw rather than making changes on any one row, or that it never assumes that the row that was clicked is still valid and instead knows what position was in the row when the button was clicked and figures out what row needs to be updated now.

Basically, avoid AsyncTasks if you can. If you can't, you need to make sure the AsyncTask doesn't touch the UI (except for calling notifyDataSetChanged), and any data it needs over the course of the task (say like a filename its saving to) are copied into local member variables of the AsyncTask at the time its created, and not read from the row view.

Answer 3

I have seen your code, and me myself code like that many times, here is my suggestion

if(position==1){

//here change color or hide view

holder.someView.setVisibility(View.Invisible);

holder.someView.setColor ==== green; //forgive me abt syntx

}else{

//Here in else part it is important that you must reverse above things
//so that when you scroll list, GREEN color do not show up in random rows

holder.someView.setVisibility(View.Visible);

holder.someView.setColor ==== red; //forgive me abt syntx

}

//same is for on click listener etc