Trouble with Adapters

I recently spent some time trying to debug an issue with an Android application I was developing. The application contained an Activity composed of a number of Fragments. Two of the fragments were list items: let’s call them listA and listB. When an item in listA is clicked, the items in listB are changed to reflect a category of items based on the listA item value. However, when refreshing the items in listB, no data was being displayed.

The Short Story

As it turns out, I was having an issue with my adapter. For my lists, I usually extend from android.widget.ArrayAdapter as it provides most of the functionality needed and I don’t have to worry about my own backing collection since the documentation states “A concrete BaseAdapter that is backed by an array of arbitrary objects.” However, after a debugging session and finally looking up the source code for the ArrayAdapter on the GitHub site, I found out that this description is not the case.

The ArrayAdpater stores the elements in a field of List<T> mObjects. This is not even an array. What it takes is the list of elements provided in the constructor or converts an array of elements using Arrays.asList(objects). In essence, because I passed my list of objects from my test data, it was being mutated by the ArrayAdapter. More specifically, when I called adapter.clear(), all my test data was being cleared.

Because of the documentation, I was under the impression that the items I provided were being copied to a backing array created by the ArrayAdapter, particularly since I was overriding the constructor that takes the objects as a list. If I were to override the constructor that takes an array of objects, I could expect that data structure to be used in the adapter. However, the constructor that takes an array then uses Arrays.asList(thearray) to convert it to a list. As a side note: since the constructor uses the Arrays.asList() to convert the provided array. If you call clear, remove or add in the adapter, it will raise an UnsupportedOperationException when trying to change the structure of the list.

When using or overriding the ArrayAdaper, try to think of it as a ListAdapter since that would better reflect the backing collection in the adapter. I have read a lot of suggestions for extending from BaseAdapter rather than ArrayAdapter. In a lot of cases, there is nothing wrong with using the ArrayAdpater and can be a convenient class to use. If you are pulling data from a SQLite database or over a data connection, you will never likely encounter any issues due to the temporary nature of the data structure created by accessing those sources. Knowing this, a temporary fix was to change my test data to return a new ArrayList of the elements. The permanent fix was to create an abstract ListAdapter that extends from BaseAdapter.

The Long Story

So I created the activity, fragments, adapters, handlers and callbacks for the inter-fragment communication. I then created some test data to make sure things were wired together correctly. When I ran the application, the listB populated as expected; however, when changing categories the elements in the list disappeared and no new elements were displayed. My initial impulse was to check for a missing notifyDataSetChanged() method call.

  1  public void setElements(List<Element> elements){
  2      this.elements = (elments == null ? new ArrayList<Element>() : elements);
  3      if (adapter != null) {
  4          adapter.clear();
  5          adapter.addAll(this.elements);
  6          adapter.notifyDataSetChanged();
  7      }
  8  }

Looking at the code in the Fragment for listB, it is obvious that is not the case. So next, I decided to see if there was an issue with the test data.

The test data was a singleton class with a map containing the groups (for listA) and the elements (for listB). The item for each map entry pointed to the same list of elements, not the best idea but for testing it seemed good enough.

The previous screen shot shows a diff of the original test data source (on the left) and some modifications made after the issue with the ArrayAdapter was identified. Essentially, I just created random groups based on an original list of elements to display. The original version is straightforward and there should not be no particular reason why no data would be displayed when changing categories (assuming the adapter maintains its own data collection).

The next thing to check is the communication between the Activity and the fragment.

private OnItemSelectedHandler<ElementGroup> 
elementGroupSelectedListener = new OnItemSelectedHandler<ElementGroup>(){
    @Override
    public void onItemSelected(ElementGroup item) {
        if(item != null) onGroupSelected(item);
     }      
};
 
private void onGroupSelected(ElementGroup group){
    Toast.makeText(this, group.getName(), Toast.LENGTH_SHORT).show();
    Fragment fragment = getFragmentManager().findFragmentByTag
	(ElementsListDialogFragment.class.getName());
    if(fragment != null) ((ElementsListDialogFragment)fragment).setElements(getElements(group));
}

The OnItemSelectedHander is bound to the Fragment for listA. When an item in the list is clicked, the event is passed back to the Activity through this method. It calls onGroupSelected with the ElementGroup. OnGroupSelected finds the fragment for listB and passes the list of elements to display.

public void setElements(List<Element> elements){
    this.elements = (elments == null ? new ArrayList<Element>() : elements);
    if (adapter != null) {
        adapter.clear();
        adapter.addAll(this.elements);
        adapter.notifyDataSetChanged();
    }
}

The setElements method in the Fragment for listB takes the new collection of elements and if there is an adapter created, clears the existing elements, then adds the new ones. I stepped through the code and monitored the variables. Prior to calling clear, elements contained 24 items. After clear was called, elements contained 24 null items. These nulls trickled all the way back up to the original test data. Thus, the adapters collection pointed to element collection provided, which of course pointed to all the lists in my test data. As such calling clear in the adapter called clear on the source list and wiped out all the data. This would not have occurred if the ArrayAdapter used its own backing collection as stated in the documentation. A quick check on GitHub for the ArrayAdapter source confirmed my suspicions.

The quick fix was to change my test data to return a copy of any group’s elements in a new list so that the clear didn’t bubble back up to the data source.

After making the change, things worked as expected because I was providing a new list to the adapter every time a category was selected.

The more permanent solution is creating my own ListAdapter to extend from.

package com.codeman;
 
import java.util.List;
 
import android.content.Context;
import android.view.View;
import android.view.ViewGroup;
import android.widget.BaseAdapter;
 
public abstract class ListAdapter<T> extends BaseAdapter {
 
    protected List<T> items;
    protected Context context;
     
    /**
     * Create a new ListAdapter using the provided List of items as the backing source.
     * If the list is mutated (items added, deleted, etc), you must call notifyDataSetChanged
     * @param context
     * @param items
     */
    public ListAdapter(Context context, List<T> items){
        this.context = context;
        this.items = items;
    }
     
    /**
     * Sets the list of items as the backing source. Calls notifyDataSetChanged
     * @param items
     */
    public void setItems(List<T> items){ 
        this.items = items; 
        notifyDataSetChanged();
    }
     
    public List<T> getItems() { return items; }
     
    @Override
    public int getCount() { return items.size(); }
 
    @Override
    public Object getItem(int position) {
        return get(position);
    }
     
    public T get(int position) { return items.get(position); }
 
    @Override
    public long getItemId(int position) {
        // TODO Auto-generated method stub
        return 0;
    }
 
    @Override
    public abstract View getView(int position, View convertView, ViewGroup parent);
 }

The implementation uses the provided list as the backing source. It also does not have any methods to mutate the list. That’s not what the adapter is for, so I believe they don’t belong in there. The list itself should be used for modifications, so any clear, add, remove or whatever is called directly on the provided list itself so it is clear where the changes are actually occurring.

Once I added the ListAdapter into the final code, a few modification were required on the listB adapter and the Fragment for listB. I was also able to change my test data so that I no longer had to return a copy of the elements for the category groups.

Diff of adapter for listB:

Diff of Fragment for listB:

Diff of test data getElements method:

As stated in the short version, there is nothing wrong with using or extending the ArrayAdapter just as long as you understand it doesn’t work as stated in the documentation. However, I am going to add my own abstract ListAdapter to my Android projects from now on.

CodeProject