ViewHolder considered harmful

Barend Garvelink

The ListView is the most complicated, common view widget in the Android SDK. It is part of a family of widgets known as Adapter Views. These are views that use an Adapter class to mediate between the view widget and the list data it is showing.

ListView, ListAdapter and List of DataType.

The adapter's job is to prepare the views to be shown in the ListView for each element in the dataset. This tends to involve many findViewById(int) lookups, which are quite costly in CPU time. The standing Best Practices for Android dictate that adapters use something called the ViewHolder Pattern to mitigate the cost of these lookups.

Adapter classes tend to be a hotspot of smelly program code in Android, because they straddle the View and Controller responsibilities of the MVC model (1). In my part of the Clean Code in Android Apps talk at XebiCon 2013 I demonstrated why adapters get smelly, why ViewHolder doesn't help and how to use a custom view to alleviate this problem. Read on for the reprise.

The problem

Consider the following application:

Screenshot of an Android app showing an address list with five contacts, all of which are presented differently depending on which data are known.

The screenshot shows a simple contact list application. The main screen is a single ListView and the list items follow just three formatting rules:

  • The email and street address fields are hidden if they have no data.
  • If there's no name in the Contact record, the email address is shown in the name field and the email address field is hidden.
  • If there's neither a name nor an email address in the Contact record, a default value is shown in the name field.

These simple rules result in a getView method on the ContactListAdapter that's already about thirty lines of code:

// https://github.com/xebia/xebicon-2013__cc-in-aa/blob/1-_naive_adapter/src/com/xebia/xebicon2013/cciaa/ContactListAdapter.java

    public View getView(int position, View convertView, ViewGroup parent) {
        final Contact item = getItem(position);
        final View view = (convertView == null)
            ? inflater.inflate(R.layout.list_item, null)
            : convertView;

        TextView nameView = ((TextView) view.findViewById(R.id.contact_name));
        if (item.getName() != null) {
            nameView.setText(item.getName());
        } else if (item.getEmail() != null) {
            nameView.setText(item.getEmail());
        } else {
            nameView.setText(R.string.unidentified);
        }

        TextView emailView = (TextView) view.findViewById(R.id.contact_email);
        if (item.getEmail() != null) {
            emailView.setText(item.getEmail());
            emailView.setVisibility(item.getName() == null ? View.GONE : View.VISIBLE);
        } else {
            emailView.setVisibility(View.GONE);
        }

        TextView addressView = (TextView) view.findViewById(R.id.contact_address);
        if (item.getAddressLines() != null) {
            addressView.setText(item.getAddressLines());
            addressView.setVisibility(View.VISIBLE);
        } else {
            addressView.setVisibility(View.GONE);
        }
        return view;
    }

Now imagine more fields in your child view, more child types in your adapter and more sets of formatting rules. The amount of formatting code grows exponentially. You can use the extract method refactoring to cut the getView method into smaller parts, but that's fighting the symptoms while ignoring the cause. The real problem is that the code is badly structured, and this is apparent when you visualize which components interact:

Dependency structure diagram showing that ListAdapter has a mixed responsibility and is directly dependent on composition elements of the view class.

The ViewHolder pattern is no solution

The preceding example does not use the ViewHolder Pattern mentioned in the introduction. The best practices of using adapter views tell you to use this pattern and it's pretty popular. A ViewHolder is a helper class that holds references to all children of your list item view. It is itself kept in the tag of the root view of each list item. This way, you have to populate these references only once. Here's a typical ViewHolder class (click to expand):

// https://github.com/xebia/xebicon-2013__cc-in-aa/blob/2-_ViewHolder_pattern/src/com/xebia/xebicon2013/cciaa/ViewHolder.java

public class ViewHolder {
    public final TextView nameView;
    public final TextView emailView;
    public final TextView addressView;

    public ViewHolder(View listItem) {
        nameView = (TextView) listItem.findViewById(R.id.contact_name);
        emailView = (TextView) listItem.findViewById(R.id.contact_email);
        addressView = (TextView) listItem.findViewById(R.id.contact_address);
        listItem.setTag( this );
    }
}

Here's what ContactListAdapter looks like when implemented using the ViewHolder Pattern (click to expand):

// https://github.com/xebia/xebicon-2013__cc-in-aa/blob/2-_ViewHolder_pattern/src/com/xebia/xebicon2013/cciaa/ContactListAdapter.java

    public View getView(int position, View convertView, ViewGroup parent) {
        ViewHolder holder;
        if (convertView == null) {
            convertView = inflater.inflate(R.layout.list_item, null);
            holder = new ViewHolder(convertView);
        } else {
            holder = (ViewHolder) convertView.getTag();
        }
        Contact item = getItem(position);
        String name = item.getName();
        String email = item.getEmail();
        String address = item.getAddressLines();
        if (name != null) {
            holder.nameView.setText(name);
        } else if (email != null) {
            holder.nameView.setText(email);
        } else {
            holder.nameView.setText(R.string.unidentified);
        }
        if (email != null) {
            holder.emailView.setText(email);
            holder.emailView.setVisibility(name == null ? View.GONE : View.VISIBLE);
        } else {
            holder.emailView.setVisibility(View.GONE);
        }
        if (address != null) {
            holder.addressView.setText(address);
            holder.addressView.setVisibility(View.VISIBLE);
        } else {
            holder.addressView.setVisibility(View.GONE);
        }
        return convertView;
    }

ViewHolder doesn't simplify the code in the adapter class. It is a performance optimization, to avoid the cost of repeated findViewById(int) lookups. A glance at the component structure reveals that it makes things more complicated:

Dependency structure diagram showing that adding a ViewHolder only complicates the object graph.

A custom ViewGroup lets you eat your cake and have it

You can do much better by using a Custom View Group for your list items. You get the benefits of ViewHolder and the clarity of your code is improved. This is all that remains of the original getView method:

// https://github.com/xebia/xebicon-2013__cc-in-aa/blob/3-_custom_ViewGroup/src/com/xebia/xebicon2013/cciaa/ContactListAdapter.java

public View getView(int position, View convertView, ViewGroup parent) {
        ContactView view;
        if (convertView == null) {
            view = (ContactView) inflater.inflate(R.layout.list_item, null);
        } else {
            view = (ContactView) convertView;
        }
        Contact item = getItem(position);
        view.showContact(item);
        return view;
    }

This gain is made by separating the View and Controller responsibilities. The new View class has a public API defined entirely in terms from your domain model, freeing the adapter class to handle only the Controller responsibility. The structure diagram reveals this simplicity:

Dependency structure diagram showing that a custom ViewGroup makes for a clean MVC separation.

How do you create a custom view group?

Creating a custom view group is just like creating a custom view. You start by creating a subclass of an existing View class. In our example the root element of list_item.xml is <LinearLayout, so we extend the android.widget.LinearLayout class. You only need to add the superclass constructors to get a functioning custom view:

public class ContactView extends LinearLayout {
    /** Inherited constructor. */
    public ContactView(Context context) {
        super(context);
    }

    /** Inherited constructor. */
    public ContactView(Context context, AttributeSet attrs) {
        super(context, attrs);
    }

    /** Inherited constructor. */
    public ContactView(Context context, AttributeSet attrs, int defStyle) {
        super(context, attrs, defStyle);
    }
}

To integrate it into the layout, simply change the root tag of the layout XML from <LinearLayout to the full class name of the custom view group, <com.xebia.xebicon2013.cciaa.ContactView. Here's layout/list_item.xml as modified to use the new custom view group:

<!-- https://github.com/xebia/xebicon-2013__cc-in-aa/blob/3-_custom_ViewGroup/res/layout/list_item.xml --></p>
<com.xebia.xebicon2013.cciaa.ContactView
        xmlns:android="http://schemas.android.com/apk/res/android"
        android:layout_width="match_parent"
        android:orientation="vertical"
        android:layout_height="match_parent">

	<!-- view group contents unchanged -->
</com.xebia.xebicon2013.cciaa.ContactView>

A custom view with only the constructors is pointless. To take advantage of the custom view class, implement the onFinishInflate() callback to look up the view references and put your application code in the rest of the class. In this case, that's the showContact(Contact) method we've seen in our earlier adapters (click to expand):

// https://github.com/xebia/xebicon-2013__cc-in-aa/blob/3-_custom_ViewGroup/src/com/xebia/xebicon2013/cciaa/ContactView.java

public class ContactView extends LinearLayout {

    public static final Contact EMPTY = new Contact(null, null, null, null);
    private TextView nameView;
    private TextView emailView;
    private TextView addressView;
    private Contact contact = EMPTY;

    /** Inherited constructors as before. */

    @Override
    protected void onFinishInflate() {
        super.onFinishInflate();
        nameView = (TextView) findViewById(R.id.contact_name);
        emailView = (TextView) findViewById(R.id.contact_email);
        addressView = (TextView) findViewById(R.id.contact_address);
    }

    public void showContact(Contact contact) {
        this.contact = (contact != null ? contact : EMPTY);
        String name = contact.getName();
        String email = contact.getEmail();
        String address = contact.getAddressLines();
        if (name != null) {
            nameView.setText(name);
        } else if (email != null) {
            nameView.setText(email);
        } else {
            nameView.setText(R.string.unidentified);
        }
        if (email != null) {
            emailView.setText(email);
            emailView.setVisibility(name == null ? View.GONE : View.VISIBLE);
        } else {
            emailView.setVisibility(View.GONE);
        }
        if (address != null) {
            addressView.setText(address);
            addressView.setVisibility(View.VISIBLE);
        } else {
            addressView.setVisibility(View.GONE);
        }
    }
}

To summarise

The custom view group approach has a number of structure advantages over the ViewHolder Pattern:

  • It exhibits higher cohesion and lower coupling than the ViewHolder approach.
  • The adapter class operates at its natural level of abstraction —entire child views—, without diving into low-level details.
  • The code no longer relies on the untyped and externally modifiable tag property of the root View.
  • The detailed conditional logic isn't gone (it's essential complexity), but it's been restricted to the narrowest possible scope. Some of this complexity could be pushed down into further custom views.
  • If the same view group is used as a child in more than one adapter, none of the low-level code needs to be copied into the new adapter.

A custom view group has only advantages to ViewHolder, because the performance benefit of avoiding unneccessary findViewById(int) lookups is preserved. The number of custom classes is the same and there's one less object instance.

Does all this mean that ViewHolder Pattern is actually Harmful? That's too harsh. Nevertheless, the benefits of using a custom ViewGroup are such that I believe it's time to retire the ViewHolder Pattern.

The full sample code for this article is on github: xebia/xebicon-2013__cc-in-aa. The three approaches are on three different branches: 1-_naieve_adapter, 2-_ViewHolder_Pattern and 3-_custom_ViewGroup.

Updated, July 30: A further refinement of this technique is published in the follow-up post A better custom ViewGroup.

1) I use "MVC" as the name of the pattern family, which includes MVP and MVVM as well as MVC. Which one Android implements is an exercise for the reader.

Comments (13)

  1. roger - Reply

    July 22, 2013 at 10:31 am

    performance diagram?

  2. Lem - Reply

    July 23, 2013 at 8:20 am

    Thanks for this great article, makes it easy to visualize this tricky Adapter stuff.

    Oh, and I think you meant "Have your cake and eat it." unless you were trying to wordplay something and I didn't catch it. ;)

  3. Manuel - Reply

    July 23, 2013 at 4:39 pm

    Isn't the title a bit exagerated? Even the author itself admits that the claim is "too harsh".

  4. David - Reply

    July 24, 2013 at 8:01 pm

    It should be noted that all of this sacrifices the reusability of views -- which in my understanding is kinda the point. I'm not saying this is bad, but you *are* introducing a strong coupling between a View[Group] and any of it's children.

    • Jerome P - Reply

      July 30, 2013 at 11:20 am

      I agree with you there is a problem with this ViewGroup pattern, because you won't be able to use this container without having the corresponding children items (in your example, 3 TextView and their ids). You introduce a strong coupling that is 'invisible' programmatically.

      Why don't you just use a custom View ?
      In your adapter, you just have to write :

      if (convertView == null) {
                  view = new ContactView(mContext);
      }
      

      Then in your list_item.xml (you may want to rename it by contactview_layout.xml) :
      replace "<com.xebia.xebicon2013.cciaa.ContactView" by "<merge".

      And finally, in the constructor of your ContactView class, inflate your layout:

      inflater.inflate(R.layout.contactview_layout, this, true);
      

      And that's it, even if you may need to set the layout orientation manually :
      setOrientation(LinearLayout.VERTICAL);
      (From my point of view, the layout width and height (both math_parent) should be set in the adapter)

      This way you can re-use the view as much as you want, and your strong coupling means something in your code.

  5. Revue de Presse Xebia | Blog Xebia France - Reply

    July 30, 2013 at 10:20 am

    [...] Garvelink, un de nos confrères chez Xebia Pays-Bas, a publié « ViewHolder Considered Harmful« , un article au titre volontairement alarmant qui nous explique pourquoi le pattern du [...]

  6. juan pablo - Reply

    December 2, 2013 at 11:29 am

    What if I want to call showContact() from inside let's say an onclickitemlistener of a listview?

    Also, shouldn't you overwrite the "convertView" variable with the converted view?

Add a Comment