On Wed, Feb 26, 2014 at 8:00 AM, Sven Meier <s...@meiers.net> wrote: > To add another point to the discussion: > > IMHO (I)ChoiceRenderer in 7.x is broken anyway. > As long as the user has to implement #equals() in his model objects *or* > override #getModelValue(), the choice renderer API is not yet complete: > > @Override > public String getModelValue() > { > final T object = getModelObject(); > if (object != null) > { > // >>> #indexOf() might be inefficient or fail depending on the > #equals() implementation <<<<< > int index = getChoices().indexOf(object); > return getChoiceRenderer().getIdValue(object, index); > } > else > { > return ""; > } > }
i am going to guess this doesnt often break because 99% of implementations out there do not use the index parameter. personally i think it should be removed from choice renderer, leave it to the user to map. we can pass the entire collection of choices into methods on choice renderer instead of passing in an index. > I wonder whether (I)ChoiceRenderer is taking the wrong direction: It's meant > to control the rendering of choices (hence its name). > Everything else could be made (or already is) adjustable via protected > methods in the choice component. it has never been just the renderer because from the beginning it is what maps objects to their ids, it only makes sense that it takes care of the inverse mapping. mapping objects to ids is usually a function of the data-store, so why have half of data-store specific code in choice renderer, and half in the component hierarchy? -igor > > Regards > Sven > > > > On 02/26/2014 04:44 PM, Martin Grigorov wrote: >> >> As Martijn explained all that is needed is: >> - s/implements/extends/ >> - remove the leading 'I' from IChoiceRenderer >> >> If the interface is preserved then all custom impls will have to add >> implementation of the new method introduced with WICKET-663. This IMO will >> lead to more work for the application developers. >> >> As I said: I am not against restoring the interface, just don't see its >> value. >> >> Martin Grigorov >> Wicket Training and Consulting >> >> >> On Wed, Feb 26, 2014 at 5:37 PM, Guillaume Smet >> <guillaume.s...@gmail.com>wrote: >> >>> On Wed, Feb 26, 2014 at 4:33 PM, tetsuo <ronald.tet...@gmail.com> wrote: >>>> >>>> Because... it's an unnecessary breaking change? >>> >>> From what I understand of your previous post, you implements some of >>> your converters in Enums, which you can do because IChoiceRenderer is >>> currently an interface. And you won't be able to do it if it's a >>> class. >>> >>> Am I right? >>> >