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;
}