Wicket
  1. Wicket
  2. WICKET-663

enhance ichoicerenderer with id->choice object lookup

    Details

    • Type: Improvement Improvement
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 1.3.0-final
    • Fix Version/s: 7.0.0
    • Component/s: None
    • Labels:
      None

      Description

      add object idtochoice(string id) to ichoicerenderer and add class choicerenderer implements ichoicerenderer that encapsulate the default linear search

      then we can remove method added in WICKET-348 to support a custom lookup

        Issue Links

          Activity

          Hide
          Martijn Dashorst added a comment -

          Moved to next milestone release.

          Show
          Martijn Dashorst added a comment - Moved to next milestone release.
          Hide
          Igor Vaynberg added a comment -

          not breaking api that badly in 1.4

          Show
          Igor Vaynberg added a comment - not breaking api that badly in 1.4
          Hide
          Juergen Donnerstag added a comment -

          Seems to be fixed in 1.5 already

          Show
          Juergen Donnerstag added a comment - Seems to be fixed in 1.5 already
          Hide
          Igor Vaynberg added a comment -

          fixed how? i dont see a reverse lookup in ichoicerenderer yet...

          Show
          Igor Vaynberg added a comment - fixed how? i dont see a reverse lookup in ichoicerenderer yet...
          Hide
          Martin Grigorov added a comment -

          Is this new functionality still needed ?
          There was no activity for more than 2 years in this ticket.

          Show
          Martin Grigorov added a comment - Is this new functionality still needed ? There was no activity for more than 2 years in this ticket.
          Hide
          Igor Vaynberg added a comment -

          something to look into in wicket 7

          Show
          Igor Vaynberg added a comment - something to look into in wicket 7
          Hide
          Martin Grigorov added a comment -

          Can you give more details/better description ?
          I can work on it in the next days.

          Show
          Martin Grigorov added a comment - Can you give more details/better description ? I can work on it in the next days.
          Hide
          Igor Vaynberg added a comment -

          the way this works currently is:

          lets say i pick a choice rendered as "17". when we need to resolve this back from the client string to the object we load up all the choices, go through them one by one, render the id using the renderer, and look for the match.

          this is ok for most cases, but when you have multiple filters on the page each with 200ish choices this may get slow because not only are you doing a linear search across all options, you are also loading all options from the database or other potentially slow storages.

          the idea behind this ticket is to make that reverse lookup ("17"->object) configurable. for large dropdowns a good optimization can be to simply cast "17" to 17 and load the object with that id from the database.

          there are two places for this kind of lookup: a method on the component and a method in the renderer.
          imho the better place is the renderer because that is the more likely place for specializations to occur and since renderer is the thing that transcodes an object into a string representation it makes it logical that it handles the inverse. for example we have an EntityChoiceRenderer that knows how to pull the database id out of entities; it makes it a logical place to override the lookup. so we would have an EntityChoiceRenderer that knows how to deal with entities in a more complete way: encode them into html and decode them efficiently.

          this would change the renderer from an interface into an abstract class, thus wicket 7.0, so it can provide the default linear lookup. maybe

          T getObject(String value, IModel<List<T>> choices)

          . im not sure if the component instance would be useful in the method signature, my feeling that most likely not really and if its needed the user can pass it in manually.

          Show
          Igor Vaynberg added a comment - the way this works currently is: lets say i pick a choice rendered as "17". when we need to resolve this back from the client string to the object we load up all the choices, go through them one by one, render the id using the renderer, and look for the match. this is ok for most cases, but when you have multiple filters on the page each with 200ish choices this may get slow because not only are you doing a linear search across all options, you are also loading all options from the database or other potentially slow storages. the idea behind this ticket is to make that reverse lookup ("17"->object) configurable. for large dropdowns a good optimization can be to simply cast "17" to 17 and load the object with that id from the database. there are two places for this kind of lookup: a method on the component and a method in the renderer. imho the better place is the renderer because that is the more likely place for specializations to occur and since renderer is the thing that transcodes an object into a string representation it makes it logical that it handles the inverse. for example we have an EntityChoiceRenderer that knows how to pull the database id out of entities; it makes it a logical place to override the lookup. so we would have an EntityChoiceRenderer that knows how to deal with entities in a more complete way: encode them into html and decode them efficiently. this would change the renderer from an interface into an abstract class, thus wicket 7.0, so it can provide the default linear lookup. maybe T getObject( String value, IModel<List<T>> choices) . im not sure if the component instance would be useful in the method signature, my feeling that most likely not really and if its needed the user can pass it in manually.
          Hide
          Martin Grigorov added a comment -

          Possible implementation is pushed to branch '663-id-to-choice-lookup'.
          Please review.

          Show
          Martin Grigorov added a comment - Possible implementation is pushed to branch '663-id-to-choice-lookup'. Please review.
          Hide
          Igor Vaynberg added a comment -

          looks good. the only other thing i would consider is nuking the interface, there is no real point to having it...

          Show
          Igor Vaynberg added a comment - looks good. the only other thing i would consider is nuking the interface, there is no real point to having it...
          Hide
          Martin Grigorov added a comment -

          Done.

          Show
          Martin Grigorov added a comment - Done.

            People

            • Assignee:
              Martin Grigorov
              Reporter:
              Igor Vaynberg
            • Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development