You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@wicket.apache.org by Martijn Dashorst <ma...@gmail.com> on 2007/07/10 23:33:20 UTC
[vote] remove type parameter from TextField constructor
Matej had the brilliant idea of auto discovery of the type parameter
for FormComponents when they use a PropertyModel or
CompoundPropertyModel. In order to remove confusion for noobs, and to
point out that there is a Better Way (tm) I propose the following
options:
For those wondering if it is still possible to set the target type:
yes you can, just use the setType() method on the form component.
I know it is a tad late in the game, but I think the clean up is
something worthwile.
[ ] remove the affected FormComponent constructors with the type parameter
[ ] deprecate the affected FormComponent constructors with the type parameter
[ ] leave them
Martijn
--
Wicket joins the Apache Software Foundation as Apache Wicket
Apache Wicket 1.3.0-beta2 is released
Get it now: http://www.apache.org/dyn/closer.cgi/wicket/1.3.0-beta2/
Re: [vote] remove type parameter from TextField constructor
Posted by Johan Compagner <jc...@gmail.com>.
it must handle string[] because sometimes for a multiselect it returns
a collection. I guess that is tricky thing to put into the converter
interface :(
On 7/11/07, Igor Vaynberg <ig...@gmail.com> wrote:
> On 7/11/07, Martijn Dashorst <ma...@gmail.com> wrote:
> >
> > On 7/11/07, Gwyn Evans <gw...@gmail.com> wrote:
> > > I did want to respond to Johan's comment, but it's /not/ a -1 vote,
> > > especially in view of your reasons above - "0" if anything.
> >
> > I didn't want to imply as such, but I would not hold a grudge if you
> > would -1 it. It is late in the game, so ...
>
>
> the thing about removing convertValue(String[])
>
> if we are going to do it it needs to be now, or it wont make it into
> 1.3.xperiod. so we need to decide if this is important enough to get
> into
> 1.3 branch.
>
> one tricky part about it is that the converter will have to be installed for
> String[]? or do we call it repeatedly if there is more then one element?
>
> -igor
>
>
> Martijn
> >
> > --
> > Wicket joins the Apache Software Foundation as Apache Wicket
> > Apache Wicket 1.3.0-beta2 is released
> > Get it now: http://www.apache.org/dyn/closer.cgi/wicket/1.3.0-beta2/
> >
>
Re: [vote] remove type parameter from TextField constructor
Posted by Igor Vaynberg <ig...@gmail.com>.
On 7/11/07, Martijn Dashorst <ma...@gmail.com> wrote:
>
> On 7/11/07, Gwyn Evans <gw...@gmail.com> wrote:
> > I did want to respond to Johan's comment, but it's /not/ a -1 vote,
> > especially in view of your reasons above - "0" if anything.
>
> I didn't want to imply as such, but I would not hold a grudge if you
> would -1 it. It is late in the game, so ...
the thing about removing convertValue(String[])
if we are going to do it it needs to be now, or it wont make it into
1.3.xperiod. so we need to decide if this is important enough to get
into
1.3 branch.
one tricky part about it is that the converter will have to be installed for
String[]? or do we call it repeatedly if there is more then one element?
-igor
Martijn
>
> --
> Wicket joins the Apache Software Foundation as Apache Wicket
> Apache Wicket 1.3.0-beta2 is released
> Get it now: http://www.apache.org/dyn/closer.cgi/wicket/1.3.0-beta2/
>
Re: [vote] remove type parameter from TextField constructor
Posted by Martijn Dashorst <ma...@gmail.com>.
On 7/11/07, Gwyn Evans <gw...@gmail.com> wrote:
> I did want to respond to Johan's comment, but it's /not/ a -1 vote,
> especially in view of your reasons above - "0" if anything.
I didn't want to imply as such, but I would not hold a grudge if you
would -1 it. It is late in the game, so ...
Martijn
--
Wicket joins the Apache Software Foundation as Apache Wicket
Apache Wicket 1.3.0-beta2 is released
Get it now: http://www.apache.org/dyn/closer.cgi/wicket/1.3.0-beta2/
Re: [vote] remove type parameter from TextField constructor
Posted by Gwyn Evans <gw...@gmail.com>.
On Wednesday, July 11, 2007, 10:53:18 AM, Martijn <ma...@gmail.com> wrote:
> On 7/11/07, Gwyn Evans <gw...@gmail.com> wrote:
>> I do - that's the reason that we /always/ slip releases, as there's
>> /always/ just one more "let's tidy this up" change.
>> It also means that committers don't feel any big urge to ship
>> releases, as they're still tweaking as they want, while normal
>> users are still waiting for a stable release they can use.
> I agree, but do have a slightly different pov: I discovered this while
> revising a chapter for the Book, and not having to explain the type
> parameter in this chapter saved about 1 1/2 page. I can now delay that
> until later in the book.
> In that light, I found the parameter to be superfluous and not needed.
> I don't mind it being shot down though: like I said, it is late in the
> game.
I did want to respond to Johan's comment, but it's /not/ a -1 vote,
especially in view of your reasons above - "0" if anything.
/Gwyn
Re: [vote] remove type parameter from TextField constructor
Posted by Martijn Dashorst <ma...@gmail.com>.
On 7/11/07, Gwyn Evans <gw...@gmail.com> wrote:
> I do - that's the reason that we /always/ slip releases, as there's
> /always/ just one more "let's tidy this up" change.
> It also means that committers don't feel any big urge to ship
> releases, as they're still tweaking as they want, while normal
> users are still waiting for a stable release they can use.
I agree, but do have a slightly different pov: I discovered this while
revising a chapter for the Book, and not having to explain the type
parameter in this chapter saved about 1 1/2 page. I can now delay that
until later in the book.
In that light, I found the parameter to be superfluous and not needed.
I don't mind it being shot down though: like I said, it is late in the
game.
Martijn
--
Wicket joins the Apache Software Foundation as Apache Wicket
Apache Wicket 1.3.0-beta2 is released
Get it now: http://www.apache.org/dyn/closer.cgi/wicket/1.3.0-beta2/
Re: [vote] remove type parameter from TextField constructor
Posted by Gwyn Evans <gw...@gmail.com>.
On Wednesday, July 11, 2007, 9:19:51 AM, Johan <jc...@gmail.com> wrote:
>>
>> Ugh. We still seem to be changing lots of fundamental stuff, don't we. :-)
>>
>> I vote:
>> [x] deprecate them for beta3
>>
>> We could possibly remove them for rc1, but that makes migration from
>> 1.2.x harder, doesn't it?
> i personally don't care that much about breaking stuff for 1.3.
> We really need to streamline the api i think for 1.3 so that we don't have
> to break a lot in the further anymore
> lets make 1.3 a very solid base..
I do - that's the reason that we /always/ slip releases, as there's
/always/ just one more "let's tidy this up" change.
It also means that committers don't feel any big urge to ship
releases, as they're still tweaking as they want, while normal
users are still waiting for a stable release they can use.
/Gwyn
Re: [vote] remove type parameter from TextField constructor
Posted by Johan Compagner <jc...@gmail.com>.
>
> Ugh. We still seem to be changing lots of fundamental stuff, don't we. :-)
>
> I vote:
> [x] deprecate them for beta3
>
> We could possibly remove them for rc1, but that makes migration from
> 1.2.x harder, doesn't it?
i personally don't care that much about breaking stuff for 1.3.
We really need to streamline the api i think for 1.3 so that we don't have
to break a lot in the further anymore
lets make 1.3 a very solid base..
Re: [vote] remove type parameter from TextField constructor
Posted by Al Maw <wi...@almaw.com>.
Martijn Dashorst wrote:
> Matej had the brilliant idea of auto discovery of the type parameter
> for FormComponents when they use a PropertyModel or
> CompoundPropertyModel. In order to remove confusion for noobs, and to
> point out that there is a Better Way (tm) I propose the following
> options:
>
> For those wondering if it is still possible to set the target type:
> yes you can, just use the setType() method on the form component.
>
> I know it is a tad late in the game, but I think the clean up is
> something worthwile.
>
> [ ] remove the affected FormComponent constructors with the type parameter
> [ ] deprecate the affected FormComponent constructors with the type
> parameter
> [ ] leave them
Ugh. We still seem to be changing lots of fundamental stuff, don't we. :-)
I vote:
[x] deprecate them for beta3
We could possibly remove them for rc1, but that makes migration from
1.2.x harder, doesn't it?
Al
--
Alastair Maw
Wicket-biased blog at http://herebebeasties.com
Re: [vote] remove type parameter from TextField constructor
Posted by Martijn Dashorst <ma...@gmail.com>.
Considering that having more than one constructor seems to scare out
noobs (take a look at DDC for a prime example), having them, when they
are not necessary, is not good for the API.
The constructor of a component is the first thing any one sees when
using it (apart from the LongAndVerboseClassNameOfTheComponent), so
not having the parameter there makes the learning curve less.
Martijn
On 7/10/07, Matej Knopp <ma...@gmail.com> wrote:
> Sorry for not voting in this reply, but do you really consider the
> constructor with type parameter confusing for noobs?
>
> -Matej
>
> On 7/10/07, Martijn Dashorst <ma...@gmail.com> wrote:
> > Matej had the brilliant idea of auto discovery of the type parameter
> > for FormComponents when they use a PropertyModel or
> > CompoundPropertyModel. In order to remove confusion for noobs, and to
> > point out that there is a Better Way (tm) I propose the following
> > options:
> >
> > For those wondering if it is still possible to set the target type:
> > yes you can, just use the setType() method on the form component.
> >
> > I know it is a tad late in the game, but I think the clean up is
> > something worthwile.
> >
> > [ ] remove the affected FormComponent constructors with the type parameter
> > [ ] deprecate the affected FormComponent constructors with the type parameter
> > [ ] leave them
> >
> > Martijn
> >
> > --
> > Wicket joins the Apache Software Foundation as Apache Wicket
> > Apache Wicket 1.3.0-beta2 is released
> > Get it now: http://www.apache.org/dyn/closer.cgi/wicket/1.3.0-beta2/
> >
>
--
Wicket joins the Apache Software Foundation as Apache Wicket
Apache Wicket 1.3.0-beta2 is released
Get it now: http://www.apache.org/dyn/closer.cgi/wicket/1.3.0-beta2/
Re: [vote] remove type parameter from TextField constructor
Posted by Matej Knopp <ma...@gmail.com>.
Sorry for not voting in this reply, but do you really consider the
constructor with type parameter confusing for noobs?
-Matej
On 7/10/07, Martijn Dashorst <ma...@gmail.com> wrote:
> Matej had the brilliant idea of auto discovery of the type parameter
> for FormComponents when they use a PropertyModel or
> CompoundPropertyModel. In order to remove confusion for noobs, and to
> point out that there is a Better Way (tm) I propose the following
> options:
>
> For those wondering if it is still possible to set the target type:
> yes you can, just use the setType() method on the form component.
>
> I know it is a tad late in the game, but I think the clean up is
> something worthwile.
>
> [ ] remove the affected FormComponent constructors with the type parameter
> [ ] deprecate the affected FormComponent constructors with the type parameter
> [ ] leave them
>
> Martijn
>
> --
> Wicket joins the Apache Software Foundation as Apache Wicket
> Apache Wicket 1.3.0-beta2 is released
> Get it now: http://www.apache.org/dyn/closer.cgi/wicket/1.3.0-beta2/
>
Re: [vote] remove type parameter from TextField constructor
Posted by Igor Vaynberg <ig...@gmail.com>.
>
> [x] deprecate the affected FormComponent constructors with the type
> parameter
>
-igor
Re: [vote] remove type parameter from TextField constructor
Posted by Johan Compagner <jc...@gmail.com>.
so you always want to call getConverter()
and if typename is not set you get a null param?
and then all the components that now implemements convertValue(String[]
values)
should override getConverted(Class) and test for null and if it is null
then return a converter which does the same as convertValue does now.
we could do that yes. It cleans up the api and makes it more obvious that
getConverter()
is always called.
johan
On 7/11/07, Al Maw <wi...@almaw.com> wrote:
>
> Al Maw wrote:
> > In my opinion, if you override getConverter() then it should always be
> > used, even if that means we have to call it with a null type parameter.
> > (Normally when you're overriding it for a particular component you don't
> > care about the type parameter anyway).
>
> Looking at this further, it seems logical to me that we should remove
> the convertValue(String[] value) method from FormComponent and push the
> logic in there into a "proper" converter, which gets registered for null
> types.
>
> Thoughts?
>
> Regards,
>
> Al
>
> --
> Alastair Maw
> Wicket-biased blog at http://herebebeasties.com
>
Re: [vote] remove type parameter from TextField constructor
Posted by Al Maw <wi...@almaw.com>.
Al Maw wrote:
> In my opinion, if you override getConverter() then it should always be
> used, even if that means we have to call it with a null type parameter.
> (Normally when you're overriding it for a particular component you don't
> care about the type parameter anyway).
Looking at this further, it seems logical to me that we should remove
the convertValue(String[] value) method from FormComponent and push the
logic in there into a "proper" converter, which gets registered for null
types.
Thoughts?
Regards,
Al
--
Alastair Maw
Wicket-biased blog at http://herebebeasties.com
Re: [vote] remove type parameter from TextField constructor
Posted by Al Maw <wi...@almaw.com>.
Johan Compagner wrote:
> this constructor can be deleted yes:
>
> public TextField(final String id, final Class type)
> {
> super(id);
> setType(type);
> }
>
>
> but i don't know about this one:
>
> public TextField(final String id, IModel model, Class type)
> {
> super(id, model);
> setType(type);
> }
>
> because if you don't use the propertymodel here then that type is pretty
> needed
But you can just call setType() yourself, which is the whole point of
this thread, I thought? (Note that we can possibly improve this further
with generic models in the future, as they are effectively like an
IObjectClassAwareModel.)
> And bigger problem here, if you override
> public IConverter getConverter(Class type)
> and you don't set the type then that converter isn't used.
This sucks. It's totally counter-intuitive.
In my opinion, if you override getConverter() then it should always be
used, even if that means we have to call it with a null type parameter.
(Normally when you're overriding it for a particular component you don't
care about the type parameter anyway).
I did a live demo at the last London Wicket event where I temporarily
overlooked this and couldn't make it work, which was silly of me. Just
goes to show it's not obvious, though. ;-)
Regards,
Al
--
Alastair Maw
Wicket-biased blog at http://herebebeasties.com
Re: [vote] remove type parameter from TextField constructor
Posted by Johan Compagner <jc...@gmail.com>.
this constructor can be deleted yes:
public TextField(final String id, final Class type)
{
super(id);
setType(type);
}
but i don't know about this one:
public TextField(final String id, IModel model, Class type)
{
super(id, model);
setType(type);
}
because if you don't use the propertymodel here then that type is pretty
needed
And bigger problem here, if you override
public IConverter getConverter(Class type)
and you don't set the type then that converter isn't used.
but this is also pure for models that don't use our own property model
variants
because we always set the type then (IObjectClassAwareModel)
johan
On 7/10/07, Martijn Dashorst <ma...@gmail.com> wrote:
>
> Matej had the brilliant idea of auto discovery of the type parameter
> for FormComponents when they use a PropertyModel or
> CompoundPropertyModel. In order to remove confusion for noobs, and to
> point out that there is a Better Way (tm) I propose the following
> options:
>
> For those wondering if it is still possible to set the target type:
> yes you can, just use the setType() method on the form component.
>
> I know it is a tad late in the game, but I think the clean up is
> something worthwile.
>
> [ ] remove the affected FormComponent constructors with the type parameter
> [ ] deprecate the affected FormComponent constructors with the type
> parameter
> [ ] leave them
>
> Martijn
>
> --
> Wicket joins the Apache Software Foundation as Apache Wicket
> Apache Wicket 1.3.0-beta2 is released
> Get it now: http://www.apache.org/dyn/closer.cgi/wicket/1.3.0-beta2/
>
Re: [vote] remove type parameter from TextField constructor
Posted by Eelco Hillenius <ee...@gmail.com>.
either
[ x ] remove the affected FormComponent constructors with the type parameter
or
[ x ] deprecate the affected FormComponent constructors with the type parameter
Eelco