[jira] [Commented] (WICKET-6132) AbstractChoice#getChoices() should be final

["Rens Verhage (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/WICKET-6132?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15223812#comment-15223812 ] 

Rens Verhage commented on WICKET-6132:
--------------------------------------

That sounds like a good solution for Wicket 7.x. I think it would be best to make the method final in Wicket 8 as it is a 'bug' that is easily re-introduced.

> AbstractChoice#getChoices() should be final
> -------------------------------------------
>
>                 Key: WICKET-6132
>                 URL: https://issues.apache.org/jira/browse/WICKET-6132
>             Project: Wicket
>          Issue Type: Improvement
>    Affects Versions: 7.2.0
>            Reporter: Rens Verhage
>         Attachments: cars-wicket6.zip, cars-wicket7.zip
>
>
> Not a bug, but a welcome improvement. The AbstractChoice#getChoices() method is non-final (the setter is though), which can cause serious application faults migrating from Wicket 6.
> I attached a Wicket 6 quickstart as well as a Wicket 7 version to reproduce the issue using two DropDownChoices.
> In Wicket 6, you could do the following with no problem. First you construct a DropDownChoice, supplying your collection of choices. Apparently, in Wicket 6, this initial collection is not directly used when converting user input. You could override the getChoices function and supply a totally different collection without any problem. You could exploit this to create a dynamic DropDownChoice that has different select options, depending on the outcome of getChoices().
> In Wicket 7 however, while converting user input, the list of choices you supplied initially is directly accessed, circumventing the getChoices method. The dynamic DropDownChoice which was working fine in Wicket 6 will most probably still work, but with surprising results...
> To reproduce, fire up supplied Wicket 6 quickstart. You'll get two DropDownChoices, make and model of cars. Check that the model drop down is showing models for both Ferrari and Chevrolet. Now, choose Ferrari as the make and for example F355 as the model. A label has been updated to show what has been selected. Now, switch the make to Chevrolet and pick the El Camino. The label will have updated accordingly and will say "Chevrolet El Camino".
> Now, try the same reproduction path in the Wicket 7 quickstart. There is no error, but the label will say "Chevrolet Testarossa". As much as I would love to see how such a mash-up of iconic cars will work out, this is obviously not what the user expected.
> Migrating from Wicket 6 to 7 can get you unexpected behavior which could've been prevented if AbstractChoice#getChoices() had been made final. The resulting compile error is much easier to fix than having to fix potentially broken data a few weeks in production...
> Note: I know that this is a totally wrong approach to creating dynamic form input components, you should always update components through their models. It's just that you've always got that piece of legacy code, written by that someone that didn't quite grasp Wicket's way. It might even have been myself as a junior developer ;)



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)