Android, List Adapter returns wrong position in getView

Question

I have found a mysterious problem that may be a bug! I have a list in my fragment. Each row has a button. List shouldn't respond to click however buttons are clickable.

In order to get which button has clicked I have created a listener and implement it in my fragment. This is the code of my adapter.

public class AddFriendsAdapter extends BaseAdapter {      public interface OnAddFriendsListener {         public void OnAddUserClicked(MutualFriends user);     }      private final String TAG = "*** AddFriendsAdapter ***";      private Context context;     private OnAddFriendsListener listener;     private LayoutInflater myInflater;     private ImageDownloader imageDownloader;     private List<MutualFriends> userList;      public AddFriendsAdapter(Context context) {         this.context = context;         myInflater = LayoutInflater.from(context);          imageDownloader = ImageDownloader.getInstance(context);     }      public void setData(List<MutualFriends> userList) {         this.userList = userList;          Log.i(TAG, "List passed to the adapter.");     }      @Override     public int getCount() {         try {             return userList.size();         } catch (Exception e) {             e.printStackTrace();             return 0;         }     }      @Override     public Object getItem(int position) {         return null;     }      @Override     public long getItemId(int position) {         return position;     }      @Override     public View getView(final int position, View convertView, ViewGroup parent) {         ViewHolder holder;          if (convertView == null) {             convertView = myInflater.inflate(R.layout.list_add_friends_row, null);             holder = new ViewHolder();              Typeface font = Typeface.createFromAsset(context.getAssets(), "fonts/ITCAvantGardeStd-Demi.ttf");             holder.tvUserName = (TextView) convertView.findViewById(R.id.tvUserName);             holder.tvUserName.setTypeface(font);             holder.ivPicture = (ImageView) convertView.findViewById(R.id.ivPicture);             holder.btnAdd = (Button) convertView.findViewById(R.id.btnAdd);             holder.btnAdd.setOnClickListener(new View.OnClickListener() {                 @Override                 public void onClick(View v) {                     Log.e(TAG, "Item: " + position);                     listener.OnAddUserClicked(userList.get(position));                 }             });              convertView.setTag(holder);         } else {             holder = (ViewHolder) convertView.getTag();         }          holder.tvUserName.setText(userList.get(position).getName());         imageDownloader.displayImage(holder.ivPicture, userList.get(position).getPhotoUrl());          return convertView;     }      public void setOnAddClickedListener(OnAddFriendsListener listener) {         this.listener = listener;     }      static class ViewHolder {         TextView tvUserName;         ImageView ivPicture;         Button btnAdd;     } } 

When I run the app, I can see my rows however since my list is long and has over 200 items when i goto middle of list and click an item then returned position is wrong (it's something like 7, sometimes 4 and etc.).

Now what is the mystery? If I active on item listener of list from my fragment and click on row then correct row position will be displayed while on that row if I click on button then wrong position will be displayed.

listView.setOnItemClickListener(new AdapterView.OnItemClickListener() {             @Override             public void onItemClick(AdapterView<?> parent, View view, int position, long id) {                 Log.e(TAG, "item " + position + " clicked.");             }         }); 

Result in logcat:

05-09 10:22:25.228: E/AddFriendsFragment(20296): item 109 clicked. 05-09 10:22:34.453: E/*** AddFriendsAdapter ***(20296): Item: 0 

Any suggestion would be appreciated. Thanks

Answer 1

Because the convertView and holder will be recycled to use, move your setOnClickListener out of the if else statement:

    if (convertView == null) {         convertView = myInflater.inflate(R.layout.list_add_friends_row, null);         holder = new ViewHolder();          Typeface font = Typeface.createFromAsset(context.getAssets(), "fonts/ITCAvantGardeStd-Demi.ttf");         holder.tvUserName = (TextView) convertView.findViewById(R.id.tvUserName);         holder.tvUserName.setTypeface(font);         holder.ivPicture = (ImageView) convertView.findViewById(R.id.ivPicture);         holder.btnAdd = (Button) convertView.findViewById(R.id.btnAdd);         convertView.setTag(holder);     } else {         holder = (ViewHolder) convertView.getTag();     }     holder.btnAdd.setOnClickListener(new View.OnClickListener() {          @Override          public void onClick(View v)                  Log.e(TAG, "Item: " + position);                 listener.OnAddUserClicked(userList.get(position));             }         }); 

It is not the best solution for that,because there will be some performance issue. I suggest you create a Map for your view and create a new view for your item then just use the relative view for each view.

I think it will be a better solution with best performance:

@Override public View getView(final int position, View convertView, ViewGroup parent) {     ViewHolder holder;      if (convertView == null) {         convertView = myInflater.inflate(R.layout.list_add_friends_row, null);         holder = new ViewHolder();          Typeface font = Typeface.createFromAsset(context.getAssets(), "fonts/ITCAvantGardeStd-Demi.ttf");         holder.tvUserName = (TextView) convertView.findViewById(R.id.tvUserName);         holder.tvUserName.setTypeface(font);         holder.ivPicture = (ImageView) convertView.findViewById(R.id.ivPicture);         holder.btnAdd = (Button) convertView.findViewById(R.id.btnAdd);         holder.btnAdd.setOnClickListener(new View.OnClickListener() {             @Override             public void onClick(View v) {                 Integer pos = (Integer)v.getTag();                 Log.e(TAG, "Item: " + pos);                 listener.OnAddUserClicked(userList.get(pos));             }         });          convertView.setTag(holder);     } else {         holder = (ViewHolder) convertView.getTag();     }      holder.tvUserName.setText(userList.get(position).getName());     imageDownloader.displayImage(holder.ivPicture, userList.get(position).getPhotoUrl());     holder.btnAdd.setTag(position);     return convertView; } 

You can also manage your view by yourself. Create every unique view for your item, don't recycle view.

//member various private Map<Integer, View> myViews = new HashMap<Integer, View>();   @Override public View getView(final int position, View convertView, ViewGroup parent) {     ViewHolder holder;     View view = myViews.get(position);     if (view == null) {         view = myInflater.inflate(R.layout.list_add_friends_row, null);         //don't need use the holder anymore.          Typeface font = Typeface.createFromAsset(context.getAssets(), "fonts/ITCAvantGardeStd-Demi.ttf");         holder.tvUserName = (TextView) convertView.findViewById(R.id.tvUserName);         holder.tvUserName.setTypeface(font);         holder.ivPicture = (ImageView) convertView.findViewById(R.id.ivPicture);         holder.btnAdd = (Button) convertView.findViewById(R.id.btnAdd);         holder.btnAdd.setOnClickListener(new View.OnClickListener() {             @Override             public void onClick(View v) {                 Integer pos = (Integer)v.getTag();                 Log.e(TAG, "Item: " + pos);                 listener.OnAddUserClicked(userList.get(pos));             }         });         holder.tvUserName.setText(userList.get(position).getName());        imageDownloader.displayImage(holder.ivPicture,                   userList.get(position).getPhotoUrl());        myViews.put(position, view);      }     return view; }