splitwise / TokenAutoComplete

Gmail style MultiAutoCompleteTextView for Android
Apache License 2.0
1.3k stars 383 forks source link

Cannot update FilteredArrayAdapter #74

Open littlefyr opened 10 years ago

littlefyr commented 10 years ago

Ok, there is an odd assumption that goes into this abstract class.

Normally, you might simply plan to call adapter.add(newItem); adapter.notifyDataSetChanged(); but the constructors store the initial data list and then pass that each time to filter.

This assumes, then, that you update the list you used to create the adapter, before calling arrayadapter (or rather that you would call add on both the adapter and list).

This is HIGHLY counter intuitive and I couldn't find anything obvious that indicates this.

Since there is no getAllItems for the ArrayAdapter a couple of approaches come mind:

Overload the modification methods to update originalObjects:

Rebuild the List by calling adapter.getItem(pos) and pass that into the filter

Modify AppFilter.performFiltering() to loop through the array directly

protected FilterResults performFiltering(CharSequence chars) {
    FilterResults result = new FilterResults();
        ArrayList<T> keptObjects = new ArrayList<T>();
    if (chars != null && chars.length() > 0) {
        String mask = chars.toString();

        for(int i=0; i<adapter.getCount; i++) {
            T object = adapter.getItem(i);
            if (keepObject(object, mask)) {
                keptObjects.add(object);
            }
        }
    } else { 
        //Not ideal
        for(int i=0; i<adapter.getCount; i++) {
            T object = adapter.getItem(i);
            keptObjects.add(object);
        }
    }
    result.count = keptObjects.size();
    result.values = keptObjects;
    return result;
}

The last is not efficient if you expect to filtering on an empty string frequently.

mgod commented 10 years ago

I'm inclined to just update the documentation to say you can't modify the list and override add*, remove, insert, clear and sort to throw exceptions if you call them. Can you provide your use case if this wouldn't be appropriate?

Making the modifications work correctly while in flight in filtering is probably more work than I'm willing to put in, but I would be happy to pull code that fixes the issue. I will note that if you do go down this route, subclassing from ArrayAdapter probably stops making sense and you should just copy the ArrayAdapter code and modify it to allow custom filtering.

littlefyr commented 10 years ago

Anytime you have an asynchronous data source, you'll run into this problem. If you were to load data from the local contacts via CursorLoader or remote data via async ajax. In my case I do both. Building the adapter once all the data has arrived is possible but from what I know, its not standard practice. The expected behaviour is that when you have an async data source for an adapter that you update the adapter.

This example from Android docs illustrates my point. If the Android tutorials show creating (and setting) the adapter when the view is initialized (onCreateView in a fragment, onCreateDialog for a DialogFragment...) and updating it when the load completes, its not just a reasonable use case, its a primary one.

If you goal is to make the list immutable then you need to make a number of additional changes:

I don't think that implementing the those methods are a big deal, however. With the exception of add(T... items) and sort, the methods are almost trivial since there is an equivalent method on List. And those other two are not difficult either. HOWEVER, if you go down this path, it's important that the internal list is ONLY changeable from these methods so you'll have to create a new internal list.

mgod commented 10 years ago

Ah, I see. In most cases, the asynchronous loaders I've seen have their own query clauses. I'd generally assume you'd use that with an asynchronous loader rather than my filter.

The FilteredArrayAdapter hijacks a lot of the existing infrastructure for it's filtering. I'd be mostly worried about implementing all of the needed methods in a way that is fast enough for general use than any particular method being hard.

littlefyr commented 10 years ago

I'm looking at this some more and I think there is a better solution overall.

I've looked at the code for ArrayAdapter and I'm not sure how useful it is to subclass it since few of the really useful things in there are exposed.

Instead, I think that it makes sense to copy the original source code (as a starting point) and implement the private filter class differently. This has the advantage of taking into account all the twiddly details like synchronization and locking which the Android devs have done.

In this case I think that it makes sense to make the objects in the adapter responsible for deciding whether they match the provided term. So an adapter defined as FilterableArrayAdapter<T extends FilterableItem> and FilterableItem as an interface that includes public boolean matches(String filterText). The filter class then simply iterates through all its members calling matches.

I think this will be faster than the current implementation, even if you don't use the FilterableItem idea, because it will avoid unnecessary overhead. And you'll get thread safety.

My FilterableItem suggestion should make creating custom filtering easier to implement and some might argue "better" if one thinks the objects should be responsible for "does this string relate to me" (i.e for the same reason you would implement java.lang.Comparable). But that is a matter of opinion.

CapitanRedBeard commented 9 years ago

@littlefyr Can you explain in more detail how I could hook up an asyn task to update my adapter in this plugin? Or possible write a pull request so mgod can update it? Thanks man!