While trying to fix the resulting compile errors of this change, I
found it hard to decipher what each parameter meant (which is why we
opted going for the fluent interface in the first place)
So take this example for instance:
addColumn(new CustomPropertyColumn<Signaal>(
new StringResourceModel(
"studentnumber",
null,
"studentnumber",
FooApp.get().getStudentTermUpperCase() + "number"
),
new StringResourceModel(
"studentnumber",
null,
"studentnumber",
FooApp.get().getStudentTermUpperCase() + "nummer"
), "event.student.studentnumber")
.setDefaultVisible(false));
addColumn(new CustomPropertyColumn<Signaal>(
new StringResourceModel(
"student",
null,
"student",
FooApp.get().getStudentTermUpperCase()
), new StringResourceModel(
"student",
null,
"student",
FooApp.get().getStudentTermUpperCase()
),
"event.student.person.fullName")
.setDefaultVisible(false));
If you need to fix the compile error, there is no guidance left for
figuring out which parameters are passed in (which was already a pain,
but you could conceivably figure it out by navigating to the relevant
constructor that Eclipse or IntelliJ was using. By breaking hard, the
transition is (perhaps needlessly) hard.
We could reïnstate the old constructors from 6.19.0 and mark them as
deprecated with the hint to refactor them into the fluent interface.
This would make transitioning (much [1]) easier.
What do you guys think?
Martijn
[1] depending on the amount of internationalisation and usage of
StringResourceModel, and whether you have crafted a wrapper around the
beast.
On Fri, May 15, 2015 at 2:10 PM, Martijn Dashorst
<[email protected]> wrote:
> I'm +1 for Guillaume's proposal, even if it breaks API late in the
> game. I'd rather have that than having to go through all
> StringResourceModel's constructors trying to figure out if they break
> silently.
>
> Martijn
>
>
> On Thu, May 14, 2015 at 8:25 PM, Guillaume Smet
> <[email protected]> wrote:
>> Hi,
>>
>> Sven's proposal was to create a new PropertyResourceModel which is IMHO
>> orthogonal to whether we decide to fix the issue we have with the new
>> StringResourceModel signature.
>>
>> Sven's proposal was to remove the MessageFormat support altogether and I
>> think it's a quite disruptive change.
>>
>> I think moving StringResourceModel to a more fluid interface would be useful
>> anyway to avoid having applications silently failing after the migration to
>> 7.
>>
>> I can work on a pull request if we agree on this.
>>
>> --
>> Guillaume
>>
>>> Le 14 mai 2015 à 08:55, Martin Grigorov <[email protected]> a écrit :
>>>
>>> Hi,
>>>
>>> I am not sure why Sven didn't push his proposal (StringResourceModel):
>>> http://markmail.org/message/35vkmmao5dxrmno2
>>> In this mail thread there was a discussion about such change (fluent API).
>>>
>>> Martin Grigorov
>>> Wicket Training and Consulting
>>> https://twitter.com/mtgrigorov
>>>
>>> On Thu, May 14, 2015 at 12:42 AM, Martijn Dashorst <
>>> [email protected]> wrote:
>>>
>>>> Sounds like a good idea. We have a special I18N class in our app
>>>> specifically as a wrapper around StringResourceModel.
>>>>
>>>> Martijn
>>>>
>>>> On Wed, May 13, 2015 at 11:29 PM, Guillaume Smet
>>>> <[email protected]> wrote:
>>>>> I gave it some more thoughts and I admit it's a bit radical but I think
>>>> we
>>>>> should simply remove all these constructors and give StringResourceModel
>>>> a
>>>>> fluid interface.
>>>>>
>>>>> We could keep 2 constructors:
>>>>> StringResourceModel(final String resourceKey, final Component component)
>>>>> StringResourceModel(final String resourceKey)
>>>>>
>>>>> and add the following methods:
>>>>> setDefaultValue
>>>>> setModel
>>>>> setParameters
>>>>> all these methods returning 'this'.
>>>>>
>>>>> We often call the constructors with null values for some of these
>>>>> parameters anyway and it would be far more readable and less error prone.
>>>>>
>>>>> When I upgrade, I really prefer having compilation errors rather than
>>>> code
>>>>> silently failing (currently it might not take the default value and it
>>>>> might get an off by one error for the parameters).
>>>>>
>>>>> Thoughts?
>>>>>
>>>>> On Wed, May 13, 2015 at 1:44 PM, Guillaume Smet <
>>>> [email protected]>
>>>>> wrote:
>>>>>
>>>>>> Yup, that's exactly the issue I had.
>>>>>>
>>>>>> It's not hard to fix in user code. The hard part is to find the
>>>>>> occurrences of the issue in the code.
>>>>>>
>>>>>>
>>>>>> On Wed, May 13, 2015 at 8:05 AM, Martin Grigorov <[email protected]>
>>>>>> wrote:
>>>>>>
>>>>>>> https://issues.apache.org/jira/browse/WICKET-5906 sounds related.
>>>>>>>
>>>>>>> Martin Grigorov
>>>>>>> Wicket Training and Consulting
>>>>>>> https://twitter.com/mtgrigorov
>>>>>>>
>>>>>>> On Mon, May 11, 2015 at 11:26 PM, Guillaume Smet <
>>>>>>> [email protected]>
>>>>>>> wrote:
>>>>>>>
>>>>>>>> Hi,
>>>>>>>>
>>>>>>>> Still working on the migration to 7.
>>>>>>>>
>>>>>>>> We used this pattern with Wicket 6:
>>>>>>>> new StringResourceModel(propertyModel.getObject(), null,
>>>>>>>> propertyModel.getObject(), objectModel, secondaryObjectModel))
>>>>>>>> which was directed to the following constructor:
>>>>>>>> StringResourceModel(key, model, defaultValue, Object... parameters)
>>>>>>>>
>>>>>>>> When migrating to 7, this pattern is silently directed to the
>>>> following
>>>>>>>> constructor:
>>>>>>>> StringResourceModel(key, model, Object... parameters)
>>>>>>>> thus the replacements are wrong.
>>>>>>>>
>>>>>>>> The fact that our application is silently broken is kinda annoying.
>>>>>>>>
>>>>>>>> I ended up moving to:
>>>>>>>> new StringResourceModel(propertyModel.getObject(), (IModel<?>) null,
>>>>>>>> propertyModel, new Object[] { objectModel, secondaryObjectModel }))
>>>>>>>>
>>>>>>>> I don't know if there is something to do about it but I thought I
>>>> might
>>>>>>> as
>>>>>>>> well get it out here.
>>>>>>>>
>>>>>>>> --
>>>>>>>> Guillaume
>>>>
>>>>
>>>>
>>>> --
>>>> Become a Wicket expert, learn from the best: http://wicketinaction.com
>>>>
>
>
>
> --
> Become a Wicket expert, learn from the best: http://wicketinaction.com
--
Become a Wicket expert, learn from the best: http://wicketinaction.com
