You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@wicket.apache.org by Jean-Baptiste Quenot <jb...@apache.org> on 2007/05/07 18:04:15 UTC
wicket-velocity : making VelocityPanel abstract
Hi team,
Thanks for adding wicket-velocity. One suggestion though, can we
make VelocityPanel abstract to let the user return the
IStringResourceStream instead of passing it in the constructor?
That would be nicer.
That would also allow to simplify the example, currently building
the template inline with StringBuffer.
This is an incompatible change, but nobody depends on it already,
right?
WDYT? See patch attached.
--
Jean-Baptiste Quenot
aka John Banana Qwerty
http://caraldi.com/jbq/
Re: wicket-velocity : making VelocityPanel abstract
Posted by Eelco Hillenius <ee...@gmail.com>.
On 5/7/07, James McLaughlin <jo...@gmail.com> wrote:
> I prefer this style of programming too, but I think making it mandatory is
> probably not a good idea. If someone isn't careful, they can too easily
> create leakage passing the panel between pages. I would vote for an
> AbstractVelocityPanel as above, and a VelocityPanel as before. This way we
> can get the best (or worst :) ) of both worlds.
That's a possibility as well. Currently, VelocityTemplate is abstract,
but has a convenience factory method:
public static VelocityPanel forTemplateResource(String id, IModel model,
final IStringResourceStream templateResource)
{
if (templateResource == null)
{
throw new IllegalArgumentException(
"argument templateResource must be not null");
}
return new VelocityPanel(id, model)
{
protected IStringResourceStream getTemplateResource()
{
return templateResource;
}
};
}
But I could live with having AbstractVelocityPanel and VelocityPanel
as well. My preference is to what we have now, but it's slight.
Eelco
Re: wicket-velocity : making VelocityPanel abstract
Posted by James McLaughlin <jo...@gmail.com>.
I prefer this style of programming too, but I think making it mandatory is
probably not a good idea. If someone isn't careful, they can too easily
create leakage passing the panel between pages. I would vote for an
AbstractVelocityPanel as above, and a VelocityPanel as before. This way we
can get the best (or worst :) ) of both worlds.
best,
jim
On 5/7/07, Eelco Hillenius <ee...@gmail.com> wrote:
>
> final IStringResourceStream stream...
> new VelocityPanel("id", model) {
> public IStringResourceStream getStringResourceStream() {
> return stream;
> }
> }
>
> Eelco
>
> On 5/7/07, Igor Vaynberg <ig...@gmail.com> wrote:
> > what if it is expensive to create it? whose job is it then to cache it?
> >
> > -igor
> >
> >
> > On 5/7/07, Eelco Hillenius <ee...@gmail.com> wrote:
> > >
> > > I actually agree with JBQ. It makes it a little bit easier to reuse
> > > classes like StringResourceStream while still being efficient (as you
> > > wouldn't keep a reference to it/ just create an instance on-the-fly).
> > >
> > > Eelco
> > >
> > >
> > > On 5/7/07, Ryan Sonnek <ry...@gmail.com> wrote:
> > > > i'm no committer, but i'm -1 for this change. just my 2 cents.
> > > >
> > > > i'd much rather use constructor arguments to ensure correct
> > > > construction than overriding methods.
> > > >
> > > > On 5/7/07, Jean-Baptiste Quenot <jb...@apache.org> wrote:
> > > > > Hi team,
> > > > >
> > > > > Thanks for adding wicket-velocity. One suggestion though, can we
> > > > > make VelocityPanel abstract to let the user return the
> > > > > IStringResourceStream instead of passing it in the constructor?
> > > > > That would be nicer.
> > > > >
> > > > > That would also allow to simplify the example, currently building
> > > > > the template inline with StringBuffer.
> > > > >
> > > > > This is an incompatible change, but nobody depends on it already,
> > > > > right?
> > > > >
> > > > > WDYT? See patch attached.
> > > > > --
> > > > > Jean-Baptiste Quenot
> > > > > aka John Banana Qwerty
> > > > > http://caraldi.com/jbq/
> > > > >
> > > > >
> > > >
> > >
> >
>
Re: wicket-velocity : making VelocityPanel abstract
Posted by Eelco Hillenius <ee...@gmail.com>.
final IStringResourceStream stream...
new VelocityPanel("id", model) {
public IStringResourceStream getStringResourceStream() {
return stream;
}
}
Eelco
On 5/7/07, Igor Vaynberg <ig...@gmail.com> wrote:
> what if it is expensive to create it? whose job is it then to cache it?
>
> -igor
>
>
> On 5/7/07, Eelco Hillenius <ee...@gmail.com> wrote:
> >
> > I actually agree with JBQ. It makes it a little bit easier to reuse
> > classes like StringResourceStream while still being efficient (as you
> > wouldn't keep a reference to it/ just create an instance on-the-fly).
> >
> > Eelco
> >
> >
> > On 5/7/07, Ryan Sonnek <ry...@gmail.com> wrote:
> > > i'm no committer, but i'm -1 for this change. just my 2 cents.
> > >
> > > i'd much rather use constructor arguments to ensure correct
> > > construction than overriding methods.
> > >
> > > On 5/7/07, Jean-Baptiste Quenot <jb...@apache.org> wrote:
> > > > Hi team,
> > > >
> > > > Thanks for adding wicket-velocity. One suggestion though, can we
> > > > make VelocityPanel abstract to let the user return the
> > > > IStringResourceStream instead of passing it in the constructor?
> > > > That would be nicer.
> > > >
> > > > That would also allow to simplify the example, currently building
> > > > the template inline with StringBuffer.
> > > >
> > > > This is an incompatible change, but nobody depends on it already,
> > > > right?
> > > >
> > > > WDYT? See patch attached.
> > > > --
> > > > Jean-Baptiste Quenot
> > > > aka John Banana Qwerty
> > > > http://caraldi.com/jbq/
> > > >
> > > >
> > >
> >
>
Re: wicket-velocity : making VelocityPanel abstract
Posted by Igor Vaynberg <ig...@gmail.com>.
what if it is expensive to create it? whose job is it then to cache it?
-igor
On 5/7/07, Eelco Hillenius <ee...@gmail.com> wrote:
>
> I actually agree with JBQ. It makes it a little bit easier to reuse
> classes like StringResourceStream while still being efficient (as you
> wouldn't keep a reference to it/ just create an instance on-the-fly).
>
> Eelco
>
>
> On 5/7/07, Ryan Sonnek <ry...@gmail.com> wrote:
> > i'm no committer, but i'm -1 for this change. just my 2 cents.
> >
> > i'd much rather use constructor arguments to ensure correct
> > construction than overriding methods.
> >
> > On 5/7/07, Jean-Baptiste Quenot <jb...@apache.org> wrote:
> > > Hi team,
> > >
> > > Thanks for adding wicket-velocity. One suggestion though, can we
> > > make VelocityPanel abstract to let the user return the
> > > IStringResourceStream instead of passing it in the constructor?
> > > That would be nicer.
> > >
> > > That would also allow to simplify the example, currently building
> > > the template inline with StringBuffer.
> > >
> > > This is an incompatible change, but nobody depends on it already,
> > > right?
> > >
> > > WDYT? See patch attached.
> > > --
> > > Jean-Baptiste Quenot
> > > aka John Banana Qwerty
> > > http://caraldi.com/jbq/
> > >
> > >
> >
>
Re: wicket-velocity : making VelocityPanel abstract
Posted by Eelco Hillenius <ee...@gmail.com>.
I actually agree with JBQ. It makes it a little bit easier to reuse
classes like StringResourceStream while still being efficient (as you
wouldn't keep a reference to it/ just create an instance on-the-fly).
Eelco
On 5/7/07, Ryan Sonnek <ry...@gmail.com> wrote:
> i'm no committer, but i'm -1 for this change. just my 2 cents.
>
> i'd much rather use constructor arguments to ensure correct
> construction than overriding methods.
>
> On 5/7/07, Jean-Baptiste Quenot <jb...@apache.org> wrote:
> > Hi team,
> >
> > Thanks for adding wicket-velocity. One suggestion though, can we
> > make VelocityPanel abstract to let the user return the
> > IStringResourceStream instead of passing it in the constructor?
> > That would be nicer.
> >
> > That would also allow to simplify the example, currently building
> > the template inline with StringBuffer.
> >
> > This is an incompatible change, but nobody depends on it already,
> > right?
> >
> > WDYT? See patch attached.
> > --
> > Jean-Baptiste Quenot
> > aka John Banana Qwerty
> > http://caraldi.com/jbq/
> >
> >
>
Re: wicket-velocity : making VelocityPanel abstract
Posted by Ryan Sonnek <ry...@gmail.com>.
i'm no committer, but i'm -1 for this change. just my 2 cents.
i'd much rather use constructor arguments to ensure correct
construction than overriding methods.
On 5/7/07, Jean-Baptiste Quenot <jb...@apache.org> wrote:
> Hi team,
>
> Thanks for adding wicket-velocity. One suggestion though, can we
> make VelocityPanel abstract to let the user return the
> IStringResourceStream instead of passing it in the constructor?
> That would be nicer.
>
> That would also allow to simplify the example, currently building
> the template inline with StringBuffer.
>
> This is an incompatible change, but nobody depends on it already,
> right?
>
> WDYT? See patch attached.
> --
> Jean-Baptiste Quenot
> aka John Banana Qwerty
> http://caraldi.com/jbq/
>
>