Wicket
  1. Wicket
  2. WICKET-348

Propose removing 'final' modifier to AbstractSingleSelectChoice.convertValue()

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 2.0 branch (discontinued)
    • Fix Version/s: 1.3.0-beta2
    • Component/s: wicket
    • Labels:
      None

      Description

      While creating a subclass of DropDownChoice, I wanted to write a specialized reverse mapping function that would allow the option value to be used as the primary key for direct lookup of the object. Unfortunately, AbstractSingleSelectChoice.convertValue() is has a 'final' modifier. (I'm guessing this is an inside joke because it does a linear search... )

      Any chance of removing the final modifier?

        Issue Links

          Activity

          Brian Topping created issue -
          Hide
          Johan Compagner added a comment -

          why can't you use IChoiceRenderer for this?

          Show
          Johan Compagner added a comment - why can't you use IChoiceRenderer for this?
          Hide
          Igor Vaynberg added a comment -

          because ichoicerenderer only does object->id, he wants a custom id->object lookup

          Show
          Igor Vaynberg added a comment - because ichoicerenderer only does object->id, he wants a custom id->object lookup
          Hide
          Johan Compagner added a comment -

          huh? IChoiceRenderer works both ways convertValue impl we now have does id-> object.

          Show
          Johan Compagner added a comment - huh? IChoiceRenderer works both ways convertValue impl we now have does id-> object.
          Hide
          Alastair Maw added a comment -

          Igor - can we close this?

          Show
          Alastair Maw added a comment - Igor - can we close this?
          Alastair Maw made changes -
          Field Original Value New Value
          Assignee Igor Vaynberg [ ivaynberg ]
          Hide
          Igor Vaynberg added a comment -

          i have factored out some stuff into convertChoiceIdToChoice that is overridable. overriding convertValue() directly would be cumbersome as you need to mess with string arrays, etc.

          basically this is useful for a direct id->choice lookup because right now our ichoicerenderer does a linear O search, and if you have 100+ entries it will get slow as you need to load each choice and then retrieve its id for comparison.

          johan what do you think? any objections? it would be nice if this was part of ichoicerenderer, then we can put the linear lookup into choicerenderer implements ichoicerenderer.

          Show
          Igor Vaynberg added a comment - i have factored out some stuff into convertChoiceIdToChoice that is overridable. overriding convertValue() directly would be cumbersome as you need to mess with string arrays, etc. basically this is useful for a direct id->choice lookup because right now our ichoicerenderer does a linear O search, and if you have 100+ entries it will get slow as you need to load each choice and then retrieve its id for comparison. johan what do you think? any objections? it would be nice if this was part of ichoicerenderer, then we can put the linear lookup into choicerenderer implements ichoicerenderer.
          Igor Vaynberg made changes -
          Status Open [ 1 ] Resolved [ 5 ]
          Resolution Fixed [ 1 ]
          Fix Version/s 1.3.0-beta2 [ 12312502 ]
          Igor Vaynberg made changes -
          Link This issue is related to WICKET-663 [ WICKET-663 ]
          Hide
          Johan Compagner added a comment -

          then the interface would be come much more poluted and harder to implement.

          Also what about ListMultipleChoice.convertValue() ?
          that works a little bit different. you can't have the convertChoiceIdToChoice but convertChoicesIdToChoices?

          i think the current way is fine (except maybe also refactor the ListMultiplyChoice.convertValue)

          Show
          Johan Compagner added a comment - then the interface would be come much more poluted and harder to implement. Also what about ListMultipleChoice.convertValue() ? that works a little bit different. you can't have the convertChoiceIdToChoice but convertChoicesIdToChoices? i think the current way is fine (except maybe also refactor the ListMultiplyChoice.convertValue)
          Hide
          Igor Vaynberg added a comment -

          i dont think the interface would become much more polluted. we would have an abstract choice renderer class to extend from instead of implementing the interface directly in case you do not care, so this change would be pretty much transparent in a lot of cases.

          i patched listmultiplechoice and added convertChoiceIdsToChoices

          Show
          Igor Vaynberg added a comment - i dont think the interface would become much more polluted. we would have an abstract choice renderer class to extend from instead of implementing the interface directly in case you do not care, so this change would be pretty much transparent in a lot of cases. i patched listmultiplechoice and added convertChoiceIdsToChoices
          Brian Topping made changes -
          Status Resolved [ 5 ] Closed [ 6 ]

            People

            • Assignee:
              Igor Vaynberg
              Reporter:
              Brian Topping
            • Votes:
              0 Vote for this issue
              Watchers:
              1 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development