Tapestry
  1. Tapestry
  2. TAPESTRY-1171

Change IAutocompleterModel semantics to be object friendly instead of string friendly

    Details

    • Type: Improvement Improvement
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 4.1.1
    • Fix Version/s: 4.1.1
    • Component/s: None
    • Labels:
      None

      Description

      The semantics of the IAutocompleterModel is unclear.
      According to bugfix of http://issues.apache.org/jira/browse/TAPESTRY-1169 the following is the scenario:
      The Objects returned in the map from filterValues(String) are toString()ed to get the label.
      The same objects are used to extract the primaryKey.
      A method should be implemented - getLabelFor( Object ).

      Now - if one uses toString() - why implement getLabelFor(Object) - which object is passed to getLabelFor() ? only the current value? why ? why not use getLabelFor() also on the map values?generic

      So, the current use of the model can be represented as :

      public interface IAutocompleteModel<VAL,KEY> // extends IPrimaryKeyConverter

      { String getLabelFor(VAL value); Map<KEY,String> filterValues(String match); @Override KEY getPrimaryKey(VAL value); @Override VAL getValue(KEY primaryKey); }

      Where it should be :

      public interface IAutocompleteModel<VAL,KEY> // extends IPrimaryKeyConverter

      { String getLabelFor(VAL value); Map<KEY,VAL> filterValues(String match); @Override KEY getPrimaryKey(VAL mapValue); @Override VAL getValue(KEY primaryKey); }

      Currently, one have to wrap entities in a custom class in order them to be used in the Autocompleter :

      I have an Autocmpleter which should search over Equities - as one could expect, there are alot of them,

      So instead of a straightforward implementation:
      public class EquityModel implements IAutocompleteModel
      {
      private OrderFacade orderFacade;

      private static final int MAX_RESULTS = 20;

      private Log log = LogFactory.getLog( EquityModel.class );

      public Map filterValues(String match)
      {
      log.debug("Searching equity for: " + match);

      Collection<Equity> list = this.orderFacade.findEquitiesByName( match );

      log.debug("Found " + list.size() + "equities" );

      Map<Long,Equity> map = new HashMap<Long,Equity>();
      if ( list == null )
      return map;

      int i = 0;
      for ( Equity e : list )

      { map.put( e.getId() , e ); if ( i++ >= MAX_RESULTS ) break; }

      return map;
      }

      public String getLabelFor(Object value)

      { return ((Equity)value).getName(); }

      public Object getPrimaryKey(Object value)

      { return ((Equity)value).getId(); }

      public Object getValue(Object primaryKey)

      { return this.orderFacade.getEquity( (Long)primaryKey ); }

      public void init() { this.orderFacade = EJBResolver.getBean( OrderFacade.class ); }

      }

      Now either I have to change the Equity.toString() method - but why should I ? what is the getLabelFor() method for ? also, the backend needs the toString() for logging - I can not just change an entity toString() just because tapestry uses it for the forntend...

      Or, I have to use a wrapper class, which is actually obsolete, if the Autocompleter would have used the map as it "should"tag:

      public class EquityModel implements IAutocompleteModel
      {
      ...

      public Map filterValues(String match)
      {
      log.debug("Searching equity for: " + match);

      Collection<Equity> list = this.orderFacade.findEquitiesByName( match );

      log.debug("Found " + list.size() + "equities" );

      Map<Long,EquityWrapper> map = new HashMap<Long,EquityWrapper>();
      if ( list == null )
      return map;

      int i = 0;
      for ( Equity e : list ) { map.put( e.getId() , new EquityWrapper(e) ); if ( i++ >= MAX_RESULTS ) break; }
      return map;
      }

      public String getLabelFor(Object value)
      { // is this ever called ? return ((Equity)value).getName(); }

      public Object getPrimaryKey(Object value)
      { return ((EquityWrapper)value).getEquity().getId(); }

      public Object getValue(Object primaryKey)
      { return this.orderFacade.getEquity( (Long)primaryKey ); }

      public void init()

      { this.orderFacade = EJBResolver.getBean( OrderFacade.class ); }

      }

        Activity

        Hide
        Jesse Kuhnert added a comment -

        Makes sense to me.

        Show
        Jesse Kuhnert added a comment - Makes sense to me.
        Hide
        Ron Piterman added a comment -

        Actually, Why return a map from filterValues ?
        Use a list and call getPK and getLabel on each member, this way you spare us using a sorted map...

        Show
        Ron Piterman added a comment - Actually, Why return a map from filterValues ? Use a list and call getPK and getLabel on each member, this way you spare us using a sorted map...

          People

          • Assignee:
            Unassigned
            Reporter:
            Ron Piterman
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development