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.

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. 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.

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().

Alternatively have all mutators-methods of a 'FeatureModels' to return a Change, which the user has to record somehow.

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.

Christian


On Mon, 5 Dec 2005 00:33:00 -0800, Igor Vaynberg <[EMAIL PROTECTED]> wrote:

ok first pass is in. filter stuff is not all there and some of it is broken, but the major idea is there. the rest should be in pretty good shape though.
its mostly backwards compatible so there should not be any problems.
feedback?

-Igor


On 12/4/05, Igor Vaynberg <[EMAIL PROTECTED]> wrote:

I think an even cleaner solution would be to turn this idea inside out.
have the dataprovider act as a locator for toolbar models, and let the
toolbars version changes themselves. instead of the locator interfaces
having only a get method they will have a set method as well, essentially they become a strongly typed IModel. yep, this is going to be clean if it
works. and we can keep total backwards compat.

so the code will look something like this
isortstatelocator { isortstate getstate(); void setstate(isortstate
state); }
SortedDataProvider implements idataprovider, isortstatelocator

then in the orderbylink onclick() {
final oldstate=locator.getstate();
addstatechange(new change() { undo() { getlocator().setstate(oldstate); }
}

i think that will work.

-Igor



On 12/4/05, Christian Essl <[EMAIL PROTECTED]> wrote:
>
> On Sat, 3 Dec 2005 13:21:15 -0800, Igor Vaynberg <
> [EMAIL PROTECTED]>
> wrote:
>
> > the problem im having is that something somewhere has to version this
> > information. if i put it back into the dataprovider then we are back
> to
> > square one.
>
> I hope that it does not cause any problems when a DataView holds the
> DataProvider in a non versioned field and the a DataProvider holds
> itself
> a reference to a component (ie an ISortStateLocator), because this is no > different than holding directly in a component a nonversioned reference
> to
> another component, which I always do in my Pages and is also done in the
>
> examples.
>
> > the problem is compounded by the fact that i want to add
> > toolbars (like a filter toolbar). this means that the dataprovider has
> > to be
> > aware of different toolbars attached to the datatable. and it would be
>
> > really nice if the toolbars did not keep their models so that you can
> add
> > two navigation toolbars and they are sharing a model stored elsewhere.
>
> Assuming that Component.addStateChange() or Page.addStateChange () were
> public (which I think it should be), I'd say have different Models for
> each feature Ie:
>
> SortStateLocator implements ISortState{
>   final Componente _comp;
>   List _sorted;
>   SortStateLocator(Component comp){
>     _comp = comp;
>   }
>   addSort(String sort){
>    ...
>    _comp.addStateChange(...sorted...);
>    ....
>   }
>   getSortList(){..}
>   getSortState(String prop){..};
> }
>
> and similar things for other toolbars:
>
> Than use it:
>
> MyPage(){
>
>    final SortStateLocator sortLoc = new SortStateLocator(this);
>    IDataProvider prov = new IDataProvider(){
>      ..
>      iterator(int from,int lenght){
>        List sortOrder = sortLoc.getSortList();
>        //use the sortOrder
>        ....
>      }
>    }
>
>    //the sortHeader which could be a prebuild comp.
>    add(new ListView("sortHeader",sortLoc.getSortList()){
>      populateItem(Item item){
>        item.add(new SortLink("link",sortLoc).add(new
> Label("name",item.getModelObjectAsString()));
>    });
>
>    //the data
>    add(new DataView("dv",prov));
>
> }
>
>
>
> I think this way you don't need a change to IDataProvider.
>
> Christian
>
>
> >
> > my original thought was to let users implement a "compound" model
> object
> > for
> > the datatable. that way all the information that needs to be versioned
> is
> > neatly in one place and all toolbars+idataprovider link to the same
> > place.
> > so a compound model object for the datatable might be something like
> this
> >
> > public class DataTableModel implements ISortStateLocator,
> > IFilterStateProvider {
> >    private MySortState state=new MySortState(); // implements
> ISortState
> >    private MyFilterState filterState=new MyFilterState();
> >
> >    public ISortState getSortState() { return state; }
> >    public Object getFilterState() { return filterState; }
> > }
> >
> > then the dataprovider can be changed to
> > IDataProvider {
> >    iterator iterator(int first, int count, Object
> datatableModelObject);
> >    int size(Object datatableModelObject);
> > }
> >
> > so that in your implementation you can do something like:
> > iterator iterator(int first, int count, Object dtmo) {
> >    ISortState sort=((ISortStateLocator)dtmo).getSortState();
> >    FilterState filter=((IFilterStateLocator)dtmo).getFIlterState();
> > }
> >
> > and maybe we can provide an uber-compound object that will have all
> > toolbar
> > models that we ship in it. it will waste a little bit of space but its
>
> > easier on the user. you can always optimize later :)
> >
> > I dont know how good that approach is, its where im headed right now
> > though.
> > If anyone has any suggestions they are welcome.
> >
> > -Igor
> >
> >
> > On 12/3/05, Christian Essl <[EMAIL PROTECTED]> wrote:
> >>
> >> I dont think that you'll need to change IDataProvider. If a concrete > >> DataProvider class would support sorting than it should just take ie
> in
> >> the constructor an ISortOrderProvider. Maybe you could even keep the
> >> ISortableDataProvider. It just implements IDataProvider and
> >> ISortOderProvider and a default implementation delgates to a
> component
> >> which implements ISortOderProvider (and which does the actual version
> >> keeping).
> >>
> >> However you know best what and how so I am happy to wait for your
> >> solution.
> >>
> >> Christian
> >>
> >> On Sat, 3 Dec 2005 11:31:59 -0800, Igor Vaynberg
> >> <[EMAIL PROTECTED]>
> >> wrote:
> >>
> >> > already on it. havent figured it all out yet. the problem is i
> think i
> >> > will
> >> > have to change IDataProvider to take extra parameters. this will
> break
> >> > all
> >> > the clients, but i dont see another way around it yet. im also
> >> > modularizing
> >> > datatable so that navigation/headers are toolbars that can be added
> or
> >> > not.
> >> > also working on a filter toolbar. should be sweet when its done,
> but
> >> > there
> >> > will be a lot of api breaks.
> >> >
> >> > -Igor
> >> >
> >> >
> >> > On 12/3/05, Christian Essl <[EMAIL PROTECTED]> wrote:
> >> >>
> >> >>
> >> >> > The data provider is kept as the model because that is where the
>
> >> >> sorting
> >> >> > state lives and that needs to be versioned. I need to refactor
> that
> >> >> out
> >> >> > of
> >> >> > the dataprovider looks like.
> >> >> >
> >> >>
> >> >> I see.
> >> >>
> >> >> Maybe an interface ISortOrderProvider which is exactly like
> >> >> ISortableDataProvider but does not extend IDataProvider. The
> >> interface
> >> >> could than be implemented by special Components directly (ie
> >> >> SortableHeader). And a DataProvider which supports sorting uses an
> >> >> instance of this interface to sort its data. I thing this would
> also
> >> >> decouple the sorting from DataView etc.
> >> >>
> >> >> Christian
> >> >>
> >> >> > -Igor
> >> >> >
> >> >> > On 12/3/05, Christian Essl < [EMAIL PROTECTED]> wrote:
> >> >> >>
> >> >> >> On Fri, 2 Dec 2005 22:51:05 +0100, Johan Compagner
> >> >> >> < [EMAIL PROTECTED]>
> >> >> >> wrote:
> >> >> >>
> >> >> >> > Maybe we should build something that it is easier for models
> to
> >> >> >> version
> >> >> >> > themselfs. Like an interface IVersionable with a method
> >> >> Serializeable
> >> >> >> > getVersionData()
> >> >> >> > Which a model can implement. And then we don't store the
> >> complete
> >> >> >> model
> >> >> >> > but
> >> >> >> > only that data.
> >> >> >>
> >> >> >> If I understand right than a IModel which does not implement
> >> >> >> IVersionable
> >> >> >> does not get stored in the version history? I think this would
> be
> >> >> good,
> >> >> >> because the current situation is confusing.
> >> >> >>
> >> >> >> Concerning DataView I'd say that when a the IDataProvider is
> given
> >> >> >> directly in the constructor. It should not be stored as the
> model
> >> of
> >> >> >> DataView but kept in an instancefield, because in nearly all
> cases
> >> it
> >> >> >> will
> >> >> >> be a pull-model. If you need versioning than you can always
> wrap
> >> it
> >> >> in
> >> >> a
> >> >> >> model.
> >> >> >>
> >> >> >> Christian
> >> >> >>
> >> >> >> >
> >> >> >> > johan
> >> >> >> >
> >> >> >> >
> >> >> >> > On 12/2/05, Nathan Hamblen <[EMAIL PROTECTED] > wrote:
> >> >> >> >>
> >> >> >> >> This came up before when I was trying to track down why
> >> reversing
> >> >> the
> >> >> >> >> sort order of a DataView was bringing down my test
> application.
> >> >> >> >> (http://thread.gmane.org/gmane.comp.java.wicket.user/4309)
> It
> >> >> turned
> >> >> >> out
> >> >> >> >> that the page versioning code was serializing the entire
> view
> >> >> >> hierarchy,
> >> >> >> >> recursively, because of anonymous model classes that
> contained
> >> >> >> pointers
> >> >> >> >> back into the view.
> >> >> >> >>
> >> >> >> >> The consensus was that you would have to turn off page
> >> versioning
> >> >> if
> >> >> >> you
> >> >> >> >> wanted an anonymous IModel. Is this still the case? I'm just
> >> now
> >> >> >> >> noticing anonymous IModels becoming sort of recommended.
> Does
> >> that
> >> >> >> mean
> >> >> >> >> than page versioning is not recommended anymore?
> >> >> >> >>
> >> >> >> >> I'll admit don't even understand how versioning is supposed
> to
> >> >> work.
> >> >> >> >> DataView sorting seems to be one of the few things that
> >> triggers
> >> >> it.
> >> >> >> >> I've got forms updating models all over the place and nary a
>
> >> >> version
> >> >> >> to
> >> >> >> >> be seen. So, I just turn it OFF then, and anonymously
> subclass
> >> >> IModel
> >> >> >> to
> >> >> >> >> my heart's content?
> >> >> >> >>
> >> >> >> >> Nathan
> >> >> >> >>
> >> >> >> >> Christian Essl wrote:
> >> >> >> >> > On Thu, 1 Dec 2005 14:24:20 -0500, Andrew Berman
> >> >> >> <[EMAIL PROTECTED]>
> >> >> >> >> > wrote:
> >> >> >> >> >
> >> >> >> >> >> Honestly, I don't think there ever was a Spring
> Integration
> >> >> >> >> problem.  I
> >> >> >> >> >> think people were just looking for a cookie-cutter
> approach
> >> to
> >> >> >> using
> >> >> >> >> >> Spring
> >> >> >> >> >> within Wicket.  It's actually quite easy to do without
> using
> >> >> any
> >> >> >> of
> >> >> >> >> the
> >> >> >> >> >> Spring stuff that Igor and others wrote, but it's always
> a
> >> good
> >> >> >> thing
> >> >> >> >> to
> >> >> >> >> >> have a common approach that everyone can follow.
> >> >> >> >> >
> >> >> >> >> > Honestly, I think Igor did a good job: It is just easier
> and
> >> >> more
> >> >> >> >> > natural to write over and over again:
> >> >> >> >> >
> >> >> >> >> > new HibernateModel(obj,_dao);
> >> >> >> >> >
> >> >> >> >> > than
> >> >> >> >> >
> >> >> >> >> > new HibernateModel(obj, new Model(){
> >> >> >> >> >    getObject(Component comp){
> >> >> >> >> >       return ((MyApplication)Appliation.get()).getDAO();
> >> >> >> >> >    }
> >> >> >> >> > });
> >> >> >> >> >
> >> >> >> >> > and this is not restricted to HibernateDAOs but to any
> bean
> >> you
> >> >> do
> >> >> >> not
> >> >> >> >> > want to get serialized whtih your components.
> >> >> >> >> >
> >> >> >> >> > Christian
> >> >> >> >> >
> >> >> >> >> >
> >> >> >> >> >
> >> >> >> >> >
> >> >> >> >> >
> >> >> >> >> >
> >> >> >> >> >
> ___________________________________________________________
> >> >> >> 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
> >> >> >> >>
> >> >> >> >>
> >> >> >> >>
> >> >> >> >> -------------------------------------------------------
> >> >> >> >> 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
> >> >> >> >>
> >> >> >>
> >> >> >>
> >> >> >>
> >> >> >> --
> >> >> >> Christian Essl
> >> >> >>
> >> >> >>
> >> >> >>
> >> >> >>
> >> >> >>
> >> >> >> ___________________________________________________________
> >> >> >> 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
> >> >> >>
> >> >>
> >> >>
> >> >>
> >> >> --
> >> >> Christian Essl
> >> >>
> >> >>
> >> >>
> >> >>
> >> >>
> >> >> ___________________________________________________________
> >> >> 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
> >> >>
> >>
> >>
> >>
> >> --
> >> Christian Essl
> >>
> >>
> >>
> >>
> >>
> >> ___________________________________________________________
> >> 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
> >>
>
>
>
> --
> Christian Essl
>
>
>
>
>
> ___________________________________________________________
> 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
>





--
Christian Essl
        

        
                
___________________________________________________________ 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

Reply via email to