So then we first have to decide what we want or better said what developers expect..
i think i am +1 for checking the params map on size.
Because if you have this:
/wicketapp/bookmarkablepage
or
/wicketapp/bookmarkablepage/x/5/y/7
and that page has a default and a params constructor? What is then the feeling what the first should do?
(my feeling says call default that is how it in java also works)
johan
On 1/14/06, Eelco Hillenius <[EMAIL PROTECTED]> wrote:
I don't think that makes things easier. Why not ALLWAYS call the
constructor with page parameters unless it isn't there in which case
we call the default constructor? That's just a very simple rule
instead of an algoritm, and thus is less error prone.
Eelco
On 1/14/06, Johan Compagner <[EMAIL PROTECTED]> wrote:
> one method is fine.
> But i think the complete flow has to be something like this
>
> boolean hasDefaultConstructor = xxx;
> boolean hasParamsConstructor = xxx;
>
> boolean pageParams = params == null || params.size () == 0;
>
> if( (pageParams)
> {
> if(hasParamsConstructor)
> {
> // call params constructor
> }
> else if(hasDefaultConstructor)
> {
> // call default
> }
> else { throw exception}
>
> }
> else
> {
> if(hasDefaultConstructor)
> {
> // call default
> }
> else if(hasParamsConstructor) {
> // call params constructor with new PageParams() empty map;
> }
> else { throw exception}
> }
>
>
>
>
> On 1/14/06, Eelco Hillenius <[EMAIL PROTECTED]> wrote:
> >
> > What I don't like about IPageFactory is that it has two methods
> > instead of just one (the one with class and page parameters). That
> > page parameter may be null - part of the contract, thus fine. And
> > constructors with the page parameter should allways precede the
> > default constructor, even if the page parameters argument is null.
> > Only when there is no pageparameters constructor should the default
> > constructor be called. Wouldn't this be a much clearer contract?
> >
> > Eelco
> >
> > On 1/14/06, Igor Vaynberg <[EMAIL PROTECTED] > wrote:
> > > hey guys, while helping pepone pepone on the user list ive come across a
> > > bug. most of the time when we instantiate a page we do it using a wrong
> call
> > > to the pagefactory. most of the time we use newPage(class, PageParams)
> even
> > > though page params are empty and so the newPage(Class) should be used
> > > instead which invokes the default constructor.
> > >
> > > i went through all the places i could find and basically did this
> > > if (pageparams.size()==0) { page=newPage(class); } else {
> > > page=newPage(class, pageparams); }
> > >
> > > and then a lot of tests broke because they depend on wicket calling the
> > > wrong constructor! so i had to go and fix all those. i would say 90% of
> all
> > > test pages were expecting to be instantiated with the wrong constructor,
> > > probably because they were copied and pasted most of the time.
> > >
> > > while doing all this and discussing it with eelco we agreed that it
> might be
> > > better to change ipagefactory to only have one constructor: (class,
> > > pageparams) and allow pagaparams to be null. the rule would be simple:
> if
> > > page params==null||pageparams.size()==0 use the default
> > > constructor, otherwise use the pageparams one.
> > >
> > >
> > > what do you think?
> > >
> > > -Igor
> > >
> > >
> > >
> >
> >
> > -------------------------------------------------------
> > 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_idv37&alloc_id865&opclick
> > _______________________________________________
> > Wicket-develop mailing list
> > [email protected]
> >
> https://lists.sourceforge.net/lists/listinfo/wicket-develop
> >
>
>
-------------------------------------------------------
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_idv37&alloc_id865&opclick
_______________________________________________
Wicket-develop mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/wicket-develop
