Now that I look at FilteredList again, I have to wonder if it is worth keeping. 
There's a lot of code there and, realistically, not that much value. Also, I 
believe it is the only remaining class that still has the potential for memory 
leaks if you forget to clear the source property (we have gotten rid of all the 
others or re-implemented them such that this isn't a problem).

I think I might advocate eliminating this class for Pivot 2.0.

G

On Aug 25, 2010, at 8:22 AM, Greg Brown wrote:

> Hi Michael,
> Can you submit your patch via JIRA? This sounds like a bug, so you can simply 
> file it as such.
> Thanks!
> Greg
> 
> On Aug 24, 2010, at 8:01 PM, Michael Allman wrote:
> 
>> There appears to be some inconsistency in FilteredList with how it behaves 
>> when the filter is null.  I think if the filter is null, the list is 
>> supposed to show all values.  In that case, the filter logic in the add() 
>> and insert() methods is incorrect.  Rather than checking
>> 
>> filter != null && filter.include(item)
>> 
>> it should be checking
>> 
>> filter == null || filter.include(item)
>> 
>> 
>> Also, the itemUpdated() method in the listListener field sometimes adds 
>> items to the list which are already there.  The problem seems to be in the 
>> following if block
>> 
>> 
>> if (comparator == null) {
>>   // Add the item to the view
>>   viewIndex = view.add(item);
>>   listListeners.itemInserted(FilteredList.this, viewIndex);
>> }
>> 
>> 
>> I modified the if statement to make a correction, but I'm not sure my patch 
>> is correct.  Here's what I replaced the if statement with
>> 
>> // Update the item in the view
>> int previousViewIndex = view.indexOf(item);
>> 
>> if (previousViewIndex == viewIndex) {
>>   // Update the item in the view
>>   view.update(viewIndex, item);
>> } else {
>>   // Remove the item from the view
>>   Sequence<T> removed = view.remove(previousViewIndex, 1);
>>   listListeners.itemsRemoved(FilteredList.this, previousViewIndex, removed);
>> 
>>   // Re-add the item to the view
>>   viewIndex = view.add(item);
>>   listListeners.itemInserted(FilteredList.this, viewIndex);
>> }
>> 
>> 
>> Again, I'm not sure this is correct but it seems to have resolved the issue 
>> I was having with duplicate items being added to the list.
>> 
>> Cheers,
>> 
>> Michael
> 

Reply via email to