You are viewing a plain text version of this content. The canonical link for it is here.
Posted to users@wicket.apache.org by Garret Wilson <ga...@globalmentor.com> on 2014/07/09 23:54:09 UTC
base pages without kludge initialization
Everyone,
Let's say I want to make a base page, extending WebPage. I want to make
it flexible so that subpages may initialize either using a model or page
parameters. So I try to do this:
protected BasePage()
{
this(null, null);
}
protected BasePage(final IModel<?> model)
{
this(null, model);
}
protected BasePage(final PageParameters parameters)
{
this(parameters, null);
}
private BasePage(final PageParameters parameters, IModel<?> model)
{
super(null, model);
//initialize stuff here
Obviously I can't do this, because several of the WebPage (and ancestor)
constructors are marked private. I'm forced to do this:
protected BasePage()
{
super();
//initialize stuff here
}
protected BasePage(final IModel<?> model)
{
super(model);
//initialize stuff here
}
protected BasePage(final PageParameters parameters)
{
super(parameters);
//initialize stuff here
}
private BasePage(final PageParameters parameters, IModel<?> model)
{
super(parameters, model);
//initialize stuff here
Look at all the duplicated initialization code! Sure, I could create a
local initialize() method that each of the methods delegate to, but not
only is that ugly it prevents me from making my class variables final
(because final variable must be initialized in the constructor).
Couldn't the parent classes be a little more lenient, allowing access to
all the constructors, making child classes meant to be base pages more
elegant?
Garret
---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscribe@wicket.apache.org
For additional commands, e-mail: users-help@wicket.apache.org
Re: base pages without kludge initialization
Posted by Martin Grigorov <mg...@apache.org>.
I'm 50/50 here.
I agree that it will make it simpler for your use case but in the same time
it will make it possible to expose the parameters+model constructor as
public in YourPage and the problem will become a RuntimeException while at
the moment it is just not possible (i.e. a compile error). Additionally the
runtime exception message will be quite wordy trying to explain why it is
not allowed to have both parameters _and_ a model.
On the other side I am not really sure that it is not allowed to have them
both. As you see in my workaround it is possible to set the model via
setDefaultModel(). So the current protection (if this is really a
protection by design) is insufficient.
Martin Grigorov
Wicket Training and Consulting
https://twitter.com/mtgrigorov
On Fri, Jul 11, 2014 at 12:05 AM, Garret Wilson <ga...@globalmentor.com>
wrote:
> On 7/10/2014 3:43 AM, Martin Grigorov wrote:
>
>>
>> I
>> think org.apache.wicket.Page#Page(org.apache.wicket.request.
>> mapper.parameter.PageParameters,
>> org.apache.wicket.model.IModel<?>) is private to make it explicit that a
>> page with PageParameters is bookmarkable and possibly stateless, and a
>> Page
>> with IModel is not bookmarkable and stateful.
>>
>
> That makes sense. But that issue can be taken care of easily enough using
> a precondition:
>
> protected BasePage(final PageParameters parameters, final IModel<?>
> model)
> {
> super(parameters);
> checkArgument(parameters == null || model == null, "Must not provide
> both parameters and a model.");
>
>
>
> There is a very simple workaround for you:
>>
>> super(parameters);
>> setDefaultModel(model);
>> this.myConstant1 = ...
>> this.myConstant2 = ...
>>
>
> Ah, thanks! I will use that.
>
> Still, it seems a shame to explicitly set the default model (which goes
> through a lot of change-reporting logic), when we could have provided it to
> the constructor. I note that even the Page constructor uses the sort of
> constructor chaining that I want to do:
>
> protected Page()
> {
> this(null, null);
> }
>
> protected Page(final IModel<?> model)
> {
> this(null, model);
> }
>
> protected Page(final PageParameters parameters)
> {
> this(parameters, null);
> }
>
> private Page(final PageParameters parameters, IModel<?> model)
> {
>
> But because the ultimate constructor is marked private, even WebPage is
> forced to go through the common-initialization drudgery:
>
> protected WebPage(final IModel<?> model)
> {
> super(model);
> commonInit();
> }
>
> etc.
>
> I recommend Wicket modify the Page ultimate constructor to be protected,
> and add a simple precondition to keep it from being accidentally misused:
>
> protected Page(final PageParameters parameters, final IModel<?> model)
> {
> super(parameters);
> checkArgument(parameters == null || model == null, "Must not provide
> both parameters and a model.");
>
>
> Cheers,
>
>
> Garret
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: users-unsubscribe@wicket.apache.org
> For additional commands, e-mail: users-help@wicket.apache.org
>
>
Re: base pages without kludge initialization
Posted by Garret Wilson <ga...@globalmentor.com>.
On 7/10/2014 3:43 AM, Martin Grigorov wrote:
>
> I
> think org.apache.wicket.Page#Page(org.apache.wicket.request.mapper.parameter.PageParameters,
> org.apache.wicket.model.IModel<?>) is private to make it explicit that a
> page with PageParameters is bookmarkable and possibly stateless, and a Page
> with IModel is not bookmarkable and stateful.
That makes sense. But that issue can be taken care of easily enough
using a precondition:
protected BasePage(final PageParameters parameters, final IModel<?>
model)
{
super(parameters);
checkArgument(parameters == null || model == null, "Must not
provide both parameters and a model.");
> There is a very simple workaround for you:
>
> super(parameters);
> setDefaultModel(model);
> this.myConstant1 = ...
> this.myConstant2 = ...
Ah, thanks! I will use that.
Still, it seems a shame to explicitly set the default model (which goes
through a lot of change-reporting logic), when we could have provided it
to the constructor. I note that even the Page constructor uses the sort
of constructor chaining that I want to do:
protected Page()
{
this(null, null);
}
protected Page(final IModel<?> model)
{
this(null, model);
}
protected Page(final PageParameters parameters)
{
this(parameters, null);
}
private Page(final PageParameters parameters, IModel<?> model)
{
But because the ultimate constructor is marked private, even WebPage is
forced to go through the common-initialization drudgery:
protected WebPage(final IModel<?> model)
{
super(model);
commonInit();
}
etc.
I recommend Wicket modify the Page ultimate constructor to be protected,
and add a simple precondition to keep it from being accidentally misused:
protected Page(final PageParameters parameters, final IModel<?> model)
{
super(parameters);
checkArgument(parameters == null || model == null, "Must not
provide both parameters and a model.");
Cheers,
Garret
---------------------------------------------------------------------
To unsubscribe, e-mail: users-unsubscribe@wicket.apache.org
For additional commands, e-mail: users-help@wicket.apache.org
Re: base pages without kludge initialization
Posted by Martin Grigorov <mg...@apache.org>.
Hi,
On Thu, Jul 10, 2014 at 12:54 AM, Garret Wilson <ga...@globalmentor.com>
wrote:
> Everyone,
>
> Let's say I want to make a base page, extending WebPage. I want to make it
> flexible so that subpages may initialize either using a model or page
> parameters. So I try to do this:
>
> protected BasePage()
> {
> this(null, null);
> }
>
> protected BasePage(final IModel<?> model)
> {
> this(null, model);
> }
>
> protected BasePage(final PageParameters parameters)
> {
> this(parameters, null);
> }
>
> private BasePage(final PageParameters parameters, IModel<?> model)
> {
> super(null, model);
> //initialize stuff here
>
I
think org.apache.wicket.Page#Page(org.apache.wicket.request.mapper.parameter.PageParameters,
org.apache.wicket.model.IModel<?>) is private to make it explicit that a
page with PageParameters is bookmarkable and possibly stateless, and a Page
with IModel is not bookmarkable and stateful.
There is a very simple workaround for you:
super(parameters);
setDefaultModel(model);
this.myConstant1 = ...
this.myConstant2 = ...
>
> Obviously I can't do this, because several of the WebPage (and ancestor)
> constructors are marked private. I'm forced to do this:
>
> protected BasePage()
> {
> super();
> //initialize stuff here
> }
>
> protected BasePage(final IModel<?> model)
> {
> super(model);
> //initialize stuff here
> }
>
> protected BasePage(final PageParameters parameters)
> {
> super(parameters);
> //initialize stuff here
> }
>
> private BasePage(final PageParameters parameters, IModel<?> model)
> {
> super(parameters, model);
> //initialize stuff here
>
> Look at all the duplicated initialization code! Sure, I could create a
> local initialize() method that each of the methods delegate to, but not
> only is that ugly it prevents me from making my class variables final
> (because final variable must be initialized in the constructor).
>
> Couldn't the parent classes be a little more lenient, allowing access to
> all the constructors, making child classes meant to be base pages more
> elegant?
>
> Garret
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: users-unsubscribe@wicket.apache.org
> For additional commands, e-mail: users-help@wicket.apache.org
>
>