BaseObservableList

[Martin Sladecek]

OK, so for the 2.1 we will introduce only this:

FXCollections.observableList(List<E> list, Callback<E, Observable[]> 
extractor);
FXCollections.observableArrayList(Callback<E, Observable[]> extractor);

These lists will fire update notifications if some of the Observables 
extracted from the elements will change.
For this, we will need a new method in ListChangeListener.Change:

public boolean wasUpdated()

returning false by default (cannot make it abstract now).

BaseObservableList + invalidation() methods will be defered to some 
later release.

Any comments? Rich, are you OK with this?

-Martin

On 12/19/2011 11:31 AM, Michael Heinrichs wrote:
> Hi Martin,
>
> as I said earlier, I do not like this approach, and I am still not 
> persuaded otherwise. :-)
>
> I think it is a rather complex implementation. (Ok, I am in an unfair 
> advantage here, because I have seen the implementation already.) My 
> feeling is, that you cannot do anything out of ordinary without 
> knowing the source code of BaseObservableList. And this is always a 
> bad sign. For example there are two patterns how to generate the 
> Change object, one for simple operations and one for complex. What is 
> a simple operation and what is a complex operation? Can an extending 
> class change the pattern that is used by default for a certain 
> operation? Why do I have to call commit() after generating a simple 
> Change and when should I call it?
>
> There is a lot of functionality in this class that is only needed for 
> writable lists. I expect almost all extensions of BaseObservableList 
> will be read-only. It seems to be a waste to have all of that 
> functionality in a class, if it is most of the time not needed. IMO 
> BaseObservableList should be optimized for the read-only case and be 
> extendible for the few writable cases. (Or we introduce a two classes 
> for both the two cases.)
>
> If you define a public class that is supposed to be extended, you 
> commit not only on the public API, but also on the implementation. My 
> feeling is, we are not at a point yet, where we can commit on the 
> implementation.
>
> Overall, I do not think we need this for any of the features that is 
> scheduled for the 2.1 release. We already discussed a good solution 
> for http://javafx-jira.kenai.com/browse/RT-15029 internally that does 
> not require the introduction of BaseObservableList. I would suggest, 
> we focus on the smaller solution for now, discuss that on the public 
> alias, and keep experimenting a little more with other options for 
> BaseObservableList after the 2.1 release.
>
> - Michael
>
>
>
> On 17.12.2011, at 08:45, Martin Sladecek wrote:
>
>> On 12/17/2011 01:22 AM, Richard Bair wrote:
>>> Hi Martin,
>>>
>>>> Hello,
>>>> I'm working on http://javafx-jira.kenai.com/browse/RT-15029 and as 
>>>> part of this effort, I'm doing a refactoring of ObservableListWrapper.
>>>>
>>>> So I've created an abstract class BaseObservableList (extends 
>>>> AbstractList) that could serve as a base class for all 
>>>> ObservableLists implementations.
>>> OK, the "Base" naming convention is one that I really regret, I wish 
>>> we had just done AbstractFoo. Oh well. The things you end up 
>>> regretting are often not the things you thought you might :-) But 
>>> the "Base" convention we're using (hopefully everywhere...) is 
>>> FooBase rather than BaseFoo, so I guess this should be 
>>> ObservableListBase.
>>
>> OK.
>>>
>>>> For a simple (modifiable) implementation, one would need to 
>>>> implement following methods:
>>>>
>>>>    public int size() {
>>>>    }
>>>>
>>>>    public E get(int index) {
>>>>    }
>>>>
>>>>    protected void doAdd(int index, E element) {
>>>>    }
>>>>
>>>>    protected E doRemove(int index) {
>>>>    }
>>>>
>>>>    protected E doSet(int index, E element) {
>>>>    }
>>>>
>>>>
>>>>
>>>> So something like ObservableListWrapper would look like this:
>>> Is the proposal also to make ObservableListWrapper public?
>>
>> Yes. I've added invalidate() methods, we were discussing before, into 
>> BaseObservableList. So making ObservableListWrapper public would also 
>> allow calling invalidate(from, to) or invalidate(index) on it.
>>>
>>>> Now when somebody would want to override something else or extend 
>>>> the implementation, an IterableChangeBuilder, that's used in 
>>>> BaseObservableList, needs to be used for this purpose.
>>>>
>>>> So when creating something by delegating to the methods from the 
>>>> List, you just need to wrap the block in
>>>> changeBuilder.beginComplexChange(); and 
>>>> changeBuilder.endComplexChange();
>>>> This builds up the change in the calls in between instead of 
>>>> calling the observers multiple times.
>>>>
>>>> E.g. setAll method works like this
>>>>
>>>>    public boolean setAll(Collection<? extends E>   col) {
>>>>        changeBuilder.beginComplexChange();
>>>>        clear(); //does not call ListChangeListeners
>>>>        addAll(col); //neither does this
>>>>        changeBuilder.endComplexChange(); //this calls the 
>>>> ListChangeListeners using just one Change object
>>>>        return true;
>>>>    }
>>>>
>>>>
>>>> On the other hand, if you want to override something simple like 
>>>> add/remove/set or use just doSet, doAdd and doRemove methods or you 
>>>> directly manipulate with your data structure, you have to use a 
>>>> different pattern and use methods from the IterableChangeBuilder 
>>>> (there are methods like nextAdd, nextRemove, nextSet). And then end 
>>>> it with commit() call.
>>>>
>>>> So, e.g. you want to implement some fast clear() (the original 
>>>> clear in AbstractList iterates over the elements and remove them 
>>>> one by one, building up the resulting change on the way), you would do:
>>>>
>>>> public void clear() {
>>>>    //do the fast clear
>>>>    changeBuilder.nextRemove(0, listOfRemovedItems);
>>>>    changeBuilder.commit(); //Calls the ListChangeListeners, but 
>>>> only if not in ComplexChange block
>>>> }
>>>>
>>>>
>>>> Of course, these 2 patterns could be combined and you can do 
>>>> something like this:
>>>>
>>>> changeBuilder.beginComplexChange();
>>>> clear(); //no call to observers, as we are in ComplexChange block.
>>>> // do some stuff using doAdd or by directly changing my data structures
>>>> chnageBuilder.nextAdd(0, x); //I've added something so I need to 
>>>> report this
>>>> changeBuilder.endComplexChange(); //calls the ListChangeListeners
>>>>
>>>>
>>>> What do you think about this API?
>>> Why is it called an IterableChangeBuilder? Is there a complete 
>>> description of the API I can see (maybe attach to the issue?).
>> We had a NonIterableChange in com.sun.javafx.collections, which did 
>> not allow iterating using next(), so this one was for creating 
>> IterableChanges. But it doesn't make much sense in javafx.collections 
>> package to call it like this, so maybe we could call it 
>> ListChangeBuilder?
>>
>> I don't have any javadoc for that yet, but I'll try to describe it 
>> briefly:
>> (package-private) IterableChangeBuilder(BaseObservableList<E> list); 
>> // Create a new builder associated with list. This is called by 
>> BaseObservableList in it's constructor
>>
>> public void beginComplexChange(); // Begins a complex change, 
>> building up a change until endComplexChangeIsCalled(). Nested complex 
>> changes are allowed.
>>
>> public void  endComplexChange(); // If this is a topmost complex 
>> change, create a Change object a call callObservers on the list.
>>
>> public void commit(); // If we are not in a complexChange, create an 
>> Change object and call callObservers
>>
>> public boolean isEmpty(); // if there's some change stored in the builder
>>
>> public void nextAdd(int from, int to); // added something on interval 
>> [from, to)
>>
>> public void nextPermutation(int from, int to, int[] perm); 
>> //permutation perm on interval [from, to)
>>
>> public void nextRemove(int idx, E removed); //single element removed 
>> from idx
>>
>> public void nextRemove(int idx, List<E> removed); //multiple elements 
>> removed from idx.
>>
>> public void nextReplace(int from, int to, List<E> removed); replaced 
>> removed with elements on interval [from, to)
>>
>> public void nextSet(int idx, E old); // changed element "old" on 
>> index "idx"
>>
>> public void nextUpdate(int pos); // updated element on position pos
>>
>> public void reset(); //builder reset
>>
>>
>> Right now, it's tightly coupled with BaseObservableList, calling 
>> directly it's callObservers method, but I'm going to separate them, 
>> so it can be used in any implementation, not just the one based on 
>> BaseObservableList.
>> Right now, the only way to use it is to use protected field in 
>> BaseObservableList.
>>
>> -Martin
>>
>>>
>>> Thanks!
>>> Richard
>>
>