On Tue, 6 Dec 2005 08:47:23 -0800, Igor Vaynberg <[EMAIL PROTECTED]>
wrote:
On 12/5/05, Christian Essl <[EMAIL PROTECTED]> wrote:
Hi,
I've just checked out the new version. Thanks for the work. Here is my
feedback:
I think the 'FeatureModels' like ISortState should record the
StateChanges
themself or at least create the Change objects themselfs.
Problem with them is recording their own state is that it becomes
non-trivial to implement your own replacements,
I don't think that you than would need your own replacements.
and also how do they know
when to restore state? and to what version? this is a bit klunky imho.
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).
Producing change objects though is something i can look into. having
something like IChangeAware that has a getChange(). This is only useful
as
an optimization though.
Imagine I have a link 'clearSort'. Than in the onClick() I'd have to call
first MultiSortState.clear(), second manually record the state-change.
This is a bit inconvienient, but the real problem comes if I use the
locator to reset an old ISortState. Than I'd have to reset it on *all*
DataProviders or any other Object which uses the ISortState instance
(ie a
SortstateLabel) and of course be sure that I can clone it.
not really true. the sort is cloned so the exact state is captured, so
you
dont have a problem where you have to call .clear() and then restore the
state. also, components should never store the state themselves, they
should
only store the locator. that way any component can rollback a state from
its
own history, and all the other components get the fresh one next time
they
pull it from the locator.
You are right the set-on-all problem is not there and you have one more
level of indirection.
Still I think it is not realy needed and has some disadvanteges:
- doubles the number of interfaces to understand, learn and implement
- never forget to always get the 'real' model from the locator
- 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)
- and IMO it is against the general idea of models that they should
themslefs be the mediator between different parts.
I just do not see a use-case or necessity for this inidirection - maybe
you can help.
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).
Apart of beeing
a lot of work I think this breaks encapsulation, because you need some
knowledge of the implementation or the versioned class to do an always
working state-recording.
i actually think its the other way around. i like the idea that my
locator/state know nothing about history, and its the components that
change
them that keep track. this bodes well with the rest of the wicket
components
that do this same thing for their own models. and components already
have a
mechanism to store/restore changes in place.
Here our opionions clash. I don't want to get in a religous discussion,
but let me explain. And if there is a public way to register a Change than
please forget the following.
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?
And third Model versioning is confusing (look at this thread) some comps
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,
because of the general pull nature).
One solution could be to have an interface IStateRecorder{void
recordState(Change change)}. The interface is directly used by the
'FeatureModel' and ie provided in the constructor. The interface can
easly
be implemented by any component just by delegating to addStateChange().
see above.
Alternatively have all mutators-methods of a 'FeatureModels' to return a
Change, which the user has to record somehow.
but what about changes that require a few calls to mutators? this
wouldnt be
too efficient.
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.
This does not mean that there should be no Locators just means that the
change production should be inside the actuall ModelImplementation.
Second I think that ISortState should have all the methods of
MultiSortState (only one setSort() and addSort()). I don't see any
reason
why SingleSortState could not implement them all. In the end it is a
MultiSortState with maxColumns 1.
MutlSortState will be back in its full glory, but i really dont think it
should be the default. How many people out there sort on more then one
column from the user interface? i think SingleSortState captures 90%
usecase, but i might be wrong. for people who want to use multistate its
just a few lines of code away. maybe you can post a vote to make
multistate
default, i just dont think that many people use it.
That's true.
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.
Thanks for the good work,
Christian
-Igor
___________________________________________________________
Gesendet von Yahoo! Mail - Jetzt mit 1GB Speicher kostenlos - Hier anmelden: http://mail.yahoo.de
-------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc. Do you grep through log files
for problems? Stop! Download the new AJAX search engine that makes
searching your log files as easy as surfing the web. DOWNLOAD SPLUNK!
http://ads.osdn.com/?ad_id=7637&alloc_id=16865&op=click
_______________________________________________
Wicket-user mailing list
Wicket-user@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/wicket-user
