As far as I know all Change(s) get recorded through the component in the
Page's versionManager which on the automatic version rewinde just executes
all the Change.undo(). Where the Change comes from, what it does and how
it get's there is not important. IMO it would work for Models as for
Components (In my case it gets indirectly through IStateRecorder to the
Page because the addStateChange() of Component is not public - and of
course the component implementing IStateRecorder should be versionable so).
i guess my biggest problem with this is that it makes the implementation of idataprovider essentially non-serializable for change tracking because it would force it to reference a component. this would make it impossible to implement setDataProvider() even if i wanted to because then the same problem would happen as before. at least my way you can serialize some data providers fine.
Still I think it is not realy needed and has some disadvanteges:
- doubles the number of interfaces to understand, learn and implement
even though the locator only has two simple methods in it.
- never forget to always get the 'real' model from the locator
this is the same as with any component in wicket. you always have to go through an imodel.
- always do the state recording yourself if you change the model - maybe
even from somewher outside a component where you do not have a ready
access to addStateChange()(ie in a static helper method)
yes i could not think of a situation where a change to the state would occur from outside a component and needs to be versioned - because then it would be a pull change and doesnt need to be versioned.
- and IMO it is against the general idea of models that they should
themslefs be the mediator between different parts.
i thought that was the whole idea - multiple components are views/work on of the same model - in this case two sorting toolbars share the same model. i think this is ok.
I just do not see a use-case or necessity for this inidirection - maybe
you can help.
and that would be the usecase from above. but i guess if we can live without having a setDataProvider() i would be willing to rethink it your way. im also not too horribly opposed against having an addChange(Change) on the page.
Apart of this the only way to record the change is to clone the whole
thing. Objects.clone() does a serialization and desirialization which is
definiately not as efficient as keeping ie an already existing SortParam.
(Please make ISortState implement ISerializable otherwise it is not
guaranteed to work).
this is a bit of a cheap shot because SortParam is immutable. If it was mutable youd have a much harder time doing SortParam old=getSortParam(); instead of SortParam old=Objects.clone(getSortParam());
First I think Component.addStateChange() should be public. In Swing (after
which this is made) it is not the component but the model which records
the state change (it emits an event) (see
http://java.sun.com/docs/books/tutorial/uiswing/components/generaltext.html#undo) .
And this is quite clear because in Swing as in Wicket the model is the
ruler of the data the component might not even know of change (In wicket
it is even worse because there are no model events).While models in Wicket
are something different to Swing this is the same.
It has also been repeatedly stated that versioning stops with component
(which is clear) and the user has to take himself care of versioning the
models - of course. But how should I write a versioned Model which has no
mean to register a Change ie if it's not an inner-class?
The only entry point to a model change is through a component as far as i can see. So it makes sense that the component is responsible for versioning any changes that occur inside its event handler.
And third Model versioning is confusing (look at this thread) some compsbecause of the general pull nature).
do it some not, some models need it some not. Update your List and than
call Component.modelChanged() and don't forget it - of course again you
need access to the component. I think there should be some way to register
Changes for Models directly and components should not track their change.
Models should track them if they need it (and IMO most don't need to,
I dont see how this would be any different. The confusion would be shifted to a different place, why does this model signal a change event why this one does not.
If you mutate the model through a componnent ie OrderByLink you'll
propably have to clone the whole thing repeatadly too, which is not
efficient.
yes it is not as efficient. the sortstate itself can produce a better change object because it can introspect itself. but what if you have to call a few method calls instead of just one? then you are producing a change object for each method call - at what point does the cloning become more efficient then keeping track of a bunch of smaller models.
Sorry if I was a bit too critical now - I always (exclusively) use your
repeater things - but I indeed think that there is one interface too much.
I like the criticism its always welcome :)
I am sending another reply with some other thoughts to keep help with the size.
