Android SimpleAdapter wrong data-item gets associated with a list row

Question

I have a listview with custom SimpleAdapter associated. I made the custom SimpleAdapter because I want to associate a different event handler with the contained textview and the imageview than the rest of the list-item widgets. So, basically I have two different event handlers for two parts of a list-item.

My SimpleAdapter customization is:

class ClickableButtonListAdapter extends SimpleAdapter {

    private static class ViewHolder {
        TextView text;
        ImageView image;
    }

    public ClickableButtonListAdapter(Context context,
        List<? extends Map<String, ?>> data, int resource, String[] from, int[] to) {
        super(context, data, resource, from, to);
    }

    @SuppressWarnings("unchecked")
    @Override
    public View getView(final int position, View convertView, ViewGroup parent) {
        final View view = super.getView(position, convertView, parent);
        ViewHolder holder = (ViewHolder) view.getTag();
        if(holder == null) {
            holder = new ViewHolder();
            holder.text = (TextView) view.findViewById(R.id.comments);
            holder.image = (ImageView) view.findViewById(R.id.arrow);
            view.setTag(holder);
            final Context context = view.getContext();
            final HashMap<String, String> article = (HashMap<String,String>) getItem(position);
            OnClickListener listener = new OnClickListener() {
                @Override
                public void onClick(View view) {
                    String item_id = article.get("item_id");
                    Intent intent = new Intent(context, HNewsCommentsActivity.class);
                    intent.putExtra("item_id", item_id);
                    context.startActivity(intent);
                }
            };
            holder.text.setOnClickListener(listener);
            holder.image.setOnClickListener(listener);
        }
        return view;
    }
}

Later, in my activity onCreate, I associate my custom SimpleAdapter like this:

    public void onCreate(Bundle savedInstanceState) {
        super.onCreate(savedInstanceState);
        setContentView(R.layout.main);
        try {
            final ArrayList<HashMap<String, String>> articles = getHNewsFeed();
            final SimpleAdapter adapter = new ClickableButtonListAdapter(this,
                    articles, R.layout.article,
                    new String[] {"title", "urlShort", "score", "comments", "item_id"},
                    new int[] {R.id.title, R.id.url, R.id.score, R.id.comments, R.id.item_id}
                    );
            final ListView l = (ListView) findViewById(android.R.id.list);
            l.setAdapter(adapter);
            l.setOnItemClickListener( new OnItemClickListener() {
                @Override
                public void onItemClick(AdapterView<?> parent, View v, int position, long id) {
                    final HashMap<String, String> article = articles.get(position);
                    String url = article.get("url");
                    Intent intent = new Intent(Intent.ACTION_VIEW, Uri.parse(url));
                    startActivity(intent);
                }
            });
        } catch (Exception e) {
            Log.w(TAG, e.getMessage());
            Toast.makeText(this, e.getMessage(), Toast.LENGTH_SHORT).show();
        }
    }

My problem is these the event handler associated with the contained textview and imageview (which I initialize in getView of the custom SimpleAdapter) seem to get a wrong data-source item and shows data of a different article upon clicking. The listview item click handler, however, gets the correct data source item. Can somebody help me in pointing out why the event handler I associate with a textview and an imageview in getView of the custom SimpleAdapter does not work correctly? My understanding is 'getItem(position)' method of the SimpleAdapter should be returning the correct data source item. But for some reason, it does not appear to do so.

Answer 1

The problem is because the article object is created outside the OnClickListener.

What you need to do is to save the item_id as a Tag in your TextView and ImageView. Add two strings in your res/strings.xml file with the IDs, tag_holder and tag_item_id. Then, change your getView() like this.

 @Override
    public View getView(final int position, View convertView, ViewGroup parent) {
        final View view = super.getView(position, convertView, parent);
        ViewHolder holder = (ViewHolder) view.getTag(R.string.tag_holder);
        if(holder == null) {
            holder = new ViewHolder();
            holder.text = (TextView) view.findViewById(R.id.comments);
            holder.image = (ImageView) view.findViewById(R.id.arrow);
            view.setTag(R.string.tag_holder, holder);
            final Context context = view.getContext();

            OnClickListener listener = new OnClickListener() {
                @Override
                public void onClick(View view) {
                    String item_id = (String)view.getTag(R.string.tag_item_id);
                    //String item_id = article.get("item_id");
                    Intent intent = new Intent(context, HNewsCommentsActivity.class);
                    intent.putExtra("item_id", item_id);
                    context.startActivity(intent);
                }
            };
            holder.text.setOnClickListener(listener);
            holder.image.setOnClickListener(listener);
        }

         final HashMap<String, String> article = (HashMap<String,String>) getItem(position);

            holder.text.setTag(R.string.tag_item_id, article.get("item_id");
            holder.image.setTag(R.string.tag_item_id, article.get("item_id");
        return view;
    }

Furthermore, I recommend NOT creating a new OnClickListener() for every list item since this OnClick listener depends only on the View Tag. Instead, instantiate your listener in the constructor of your adapter, and use that same instance for all the List items.

Answer 2

There is a mistake in your logic. Adapter's getView method is called a lot of times and one View can show different items in different periods of time.

So how it works:

When ListView wants an item it calls method getView from Adapter class (doesn't matter if this view should be shown to user or used just to calculate view item's size). ListView passes convertView parameter - this is old view that this adapter returned some time ago and now not needed. It is ok to reuse it and not inflate again (super method works like this). And here is your code problem. If old view is passed to your getView method, it already has tag object. And your (holder == null) == false. And so view that represents one article opens an other.

Try to change you logic to fix this mistake. The best way is to create holder only one time and set view listeners every time.

Hope this would help.

EDIT. Here is two ways to fix problem.

@SuppressWarnings("unchecked")
@Override
public View getView(final int position, View convertView, ViewGroup parent) {
    final View view = super.getView(position, convertView, parent);
    ViewHolder holder = (ViewHolder) view.getTag();
    if(holder == null) {
        holder = new ViewHolder();
        holder.text = (TextView) view.findViewById(R.id.comments);
        holder.image = (ImageView) view.findViewById(R.id.arrow);
        view.setTag(holder);
    }

    final Context context = view.getContext();
    final HashMap<String, String> article = (HashMap<String,String>) getItem(position);
    OnClickListener listener = new OnClickListener() {
        @Override
        public void onClick(View view) {
            String item_id = article.get("item_id");
            Intent intent = new Intent(context, HNewsCommentsActivity.class);
            intent.putExtra("item_id", item_id);
            context.startActivity(intent);
        }
    };
    holder.text.setOnClickListener(listener);
    holder.image.setOnClickListener(listener);

    return view;
}

The second, that I love more. Thanks Vinay S Shenoy for this version.

@SuppressWarnings("unchecked")
@Override
public View getView(final int position, View convertView, ViewGroup parent) {
    final HashMap<String, String> article = (HashMap<String,String>) getItem(position);

    final View view = super.getView(position, convertView, parent);
    ViewHolder holder = (ViewHolder) view.getTag();
    if(holder == null) {
        holder = new ViewHolder();
        holder.text = (TextView) view.findViewById(R.id.comments);
        holder.image = (ImageView) view.findViewById(R.id.arrow);
        view.setTag(holder);
        final Context context = view.getContext();
        OnClickListener listener = new OnClickListener() {
            @Override
            public void onClick(View view) {
                String item_id = view.getTag();
                //String item_id = article.get("item_id");
                Intent intent = new Intent(context, HNewsCommentsActivity.class);
                intent.putExtra("item_id", item_id);
                context.startActivity(intent);
            }
        };
        holder.text.setOnClickListener(listener);
        holder.image.setOnClickListener(listener);
    }

    holder.text.setTag(article.get("item_id"));
    holder.image.setTag(article.get("item_id"));

    return view;
}