About ViewHolder pattern implementation optimisati

Question

So the well-known ViewHolder pattern using usually looks like (ListAdapter):

    ...

    @Override
    public View getView(final int position, View convertView, final ViewGroup parent) {

        final Album album = albums.get(position);

        ViewHolder viewHolder = null;
        if (convertView==null){
            convertView = inflater.inflate(R.layout.albums_list_item, null);

            final ImageView albumImage = (ImageView) convertView.findViewById(R.id.album_icon);

            final TextView txtTitle = (TextView) convertView.findViewById(R.id.album_title);

            final TextView txtDescription = (TextView) convertView.findViewById(R.id.album_copyright);

            viewHolder = new ViewHolder();
            viewHolder.albumImage = albumImage;
            viewHolder.txtTitle = txtTitle;
            viewHolder.txtDescription = txtDescription;
            convertView.setTag(viewHolder);
        }
        else
            viewHolder = (ViewHolder)convertView.getTag();

        viewHolder.txtTitle.setText(album.getTitle(locale));
        viewHolder.txtDescription.setText(album.getCopyrightInfo(locale));
        ...
        return convertView;
    }

while the ViewHolder class is usually looks like:

static class ViewHolder{
    public ImageView previewImage;
    public TextView txtTitle;
    public TextView txtDescription;
}

My questions is about ViewHolder implementation.
1) Why doesn't it use a constructor instead of initializing every single field?
2) Why does it use the default access type instead of protected (actually it has to be private but this impacts performance due to static accessors created byt JIT)? Well, I guess its about inheritance only.
So why the following pattern is not better (excluding "protected vs default" access type):

protected static class ViewHolder{
    public final ImageView previewImage;
    public final TextView txtTitle;
    public final TextView txtDescription;

    public ViewHolder (final ImageView previewImage,  final TextView txtTitle, final TextView txtDescription){
        this.previewImage = previewImage;
        this.txtTitle = txtTitle;
        this.txtDescription = txtDescription;
    }
}

and the only change in ListAdapter is:

...
final TextView txtDescription = (TextView) convertView.findViewById(R.id.album_copyright);
viewHolder = new ViewHolder(albumImage, txtTitle, txtDescription);
convertView.setTag(viewHolder);
...

Anyway it must call a constructor. Is it just a matter of taste? Or is this version slower somehow or does it impact performance in some way?

Answer 1

I use an approach very similar to yours but I take it one step further, because the ViewHolder is private to the adapter class I tightly couple it to the class by passing in the view to it's constructor and setting the values there e.g.

    private class ViewHolder
    {
      protected final ImageView image;
      protected final TextView  title;
      protected final TextView  status;

      public ViewHolder( final View root )
      {
         image = (ImageView) root.findViewById( R.id.artist_image );
         title = (TextView) root.findViewById( R.id.artist_title );
         status = (TextView) root.findViewById( R.id.artist_status );
      }
   }

And in getView(...)

 View row = convertView;

  if ( null == row || null == row.getTag() )
  {
     row = inflater.inflate( R.layout.adapter_artists, null );
     holder = new ViewHolder( row );
     row.setTag( holder );
  }
  else
  {
     holder = (ViewHolder) row.getTag();
  }

I like doing it this way because it keeps my adapter code simpler in getView(...) and has the benefit of final variables. I'd probably get a minor speed boost making it protected instead but I find the performance is adequate even with huge lists.

Answer 2

I thinks that it is just mater of taste. As for me it is even looks better then standard one. Also your version probably will be potentially faster because of using final variables.

Answer 3

In my opinion this is the best way doing that, but there is something which I would change i your code. You have a constructor in your ViewHolder where you are setting the views, but as I can see you are not using it in your code. I would use it or just remove it. And one anothet thing, actually there is a better way to get the same effect, but it will work only on Android 4+ :

@Override
public View getView(int position, View convertView, ViewGroup parent) {

    ImageView mIcon;
    TextView mName;
    if (convertView == null) {
        convertView = LayoutInflater.from(context)
          .inflate(R.layout.my_contact_listing, parent, false);
        mIcon = (ImageView) convertView.findViewById(R.id.contact_icon);
        mName = (TextView) convertView.findViewById(R.id.contact_name);
        convertView.setTag(R.id.contact_icon, mIcon);
        convertView.setTag(R.id.contact_name, mName);
    } else {
        mIcon = (ImageView) convertView.getTag(R.id.contact_icon);
        mName = (TextView) convertView.getTag(R.id.contact_name);
    }

    Contact mContact = getItem(position);
    mName.setText(mContact.getName());
    mIcon.setImageResource(mContact.getIcon());

    return convertView;
}