cocoon-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jonas Ekstedt <ekst...@ibg.uu.se>
Subject Re: [RT] Attribute Rendering and CForms Convertors
Date Mon, 15 Nov 2004 06:56:20 GMT
On Sun, 2004-11-14 at 22:57 +0100, Daniel Fagerstrom wrote:
> Jonas Ekstedt wrote:
> > On Wed, 2004-11-10 at 12:15 +0100, Daniel Fagerstrom wrote:  
> >>CForms Convertor Integration
> >>----------------------------
> <snip/>
> > How about an interface like this:
> > 
> > public interface Convertor {
> > 
> >     public String getId();
> >     public void setId(String id);
> >     public Class getTypeClass();
> >     public String convertToString(Object value, Locale locale) 
> > 	throws Exception;
> >     public Object convertFromString(String value, Locale locale) 
> > 	throws Exception;
> > 
> > }
> > 
> > Each convertor has an id (useful for error reporting when conversion
> > fails) and a typeClass (ie the class type it converts to/from). Those
> > convertors that need extra configuration would implement Configurable.
> 
> Looks good to me. Some small comments: Do we need an Exception for 
> convertToString, can that go wrong? Shouldn't we use a more specialized 
> Exception, conversion failure should gennerally be checked I guess.

I just threw that Exception in there for symmetry ;) it probably isn't
all that useful.

>  Is 
> setId necessary, or are you preparing for dependecy injection?

I think there are cases when supplying a convertor with an id can be
useful, especially when constructing error messages if conversions
failed. If you look at the conversion block that I posted to bugzilla a
few days ago http://issues.apache.org/bugzilla/show_bug.cgi?id=32223
you can see how my additions to CForm uses convertor.getId() to
construct meaningful ValidationErrors (the GlobalConvertor is the glue
between the conversion block and CForm).

>>From o.a.c.forms.datatype.convertor.GlobalConvertor

public class GlobalConvertor implements Convertor {
  ...
  public ConversionResult convertFromString(String value, Locale locale,
Convertor.FormatCache dummy) {
       try {
           return new ConversionResult(convertor.convertFromString(value, locale));
       } catch (Exception e) {
           return new ConversionResult(new ValidationError("conversion-failed." + convertor.getId()));
       }
   }
   ...
}

The reason I put in setId as well was that rather than having all
Convertors implement Configurable and get their id that way, they can be
fed the id by the ConvertorManager at instantiation.

<snip/>
> Thinking of it I would prefer:
> 
> public Convertor getConvertor(Class c);
> public Convertor getConvertor(Class c, Locale l);
> public Convertor getConvertor(Class c, Locale l, String type);
> 
> In this case we would remove Locale form the converToString and 
> convertFromString methods in the Convertor interface. That would make 
> the converters less similar to CForms converters that might be bad. But 
> it would alow more control over fallback handling in the 
> ConvertorManager. It would be easier to use also I think.

Agree, locale in getConvertor() rather than in convertToString seems
more reasonable.

> Now, it wouldn't be that hard to reimplement the CForms convertors in 
> terms of these simpler convertors. Or to refactor CForms using this kind 
> of convertors, the convertors are not used in that many places in the 
> code. As long as the form definition files doesn't change I would 
> believe that rather few would be affected.

In the conversion block mentioned above I included patches for CForm
that enables form definitions to use convertors in the conversion block
instead. These new convertors can be used alongside the old convertors
from CForm. In the samples provided I have for example:

<fd:field id="date">
 <fd:datatype base="date">
  <fd:convertor type="global" refid="java.util.Date#short"/>
 </fd:datatype>
</fd:field>

The "global" convertor is a normal CForm convertor but it calls the
ConvertorManager for a convertor with id = refid when it is
instantiated.

>  Hopefully some of the more 
> active CForms developers/users can comement on that.


> 
> Anyway for the moment I think we should strive for simplicity and solve 
> CForms compability issues later.
> 
> > One problem with this type of configuration would be that it is hard to
> > implement a fallback mechanisms (eg if we don't have a
> > "java.util.Date#full" converter use "java.util.Date"). However I don't
> > know how useful fallback would be anyway (see below).
> 
> I rather get a date in default locale and form than an exception if I 
> use an webapp that not is localized for swedish use ;)

I agree that locale fallback is necessary. However shouldn't the
fallback mechanism be implemented in the Convertors themselves rather
than in the ConvertorManager that doles out the convertors. Some
Convertors will do fallback, others would throw exceptions and those
that are locale insensitive would ignore the issue altogether.

> 
> >>What lacks in CForms Convertor from our POV, is a way to use 
> >>presentation classes. Maybe also would like to have more general 
> >>convertion between objects in general. I have no use cases for that and 
> >>I would prefer keeping it as simple as possible.
> > 
> > One possible use case for other types of conversion would be Iterator
> > conversion. This could be useful in forEach template tags. Eg:
> > 
> > <forEach var="widget" items="$repeaterWidget">
> >   ...
> > </forEach>
> > 
> > Then you could use forEach for any object type that has an Object <->
> > Iterator convertor.
> 
> Hmm... seem like overkill to use a locale and type aware conversion 
> mechanism for that.

Yeah, reading that bit again, it does seem overly creative.

> 
> >>Presentation Classes
> >>--------------------
> >>
> >>We need to find a good syntax for presentation classes. I'm not 
> >>completely convinced about the '#' selector, as it normally is connected 
> >>to positions, but I don't have any better suggestions. The 
> >>${date?class=short} is unecesarilly verbose as class is the only 
> >>attribute that we use. Using specialized tags might also be an 
> >>alternative. But that is rather clumsy for xml attributes, and it would 
> >>be nicer to have a mechanism that is not connected to a certain template 
> >>language.
> > 
> > 
> > I think the choice of separator character is rather limited as it cannot
> > be either a legal variable name character nor a legal operator
> > character. Someone might want to do ${1+1#decimal}.
> 
> "#" will be fine, I'm starting to get used to it and have not seen any 
> better alternatives. I also believe that it will mainly be a concern for 
> the user of the convertor, the template language e.g.
> 
> >>How does type, locale and class interact? I would propose to have the 
> >>priority order type > locale > class. Meaning that if type, locale and

> >>class are given the convertion component will first look for convertions 
> >>having the right type, if none is given a default convertor (.toString) 
> >>is used. If there are convertors of the right type but not the right 
> >>locale, the default convertor for the type is used. If there are 
> >>convertors of the right type and locale, but not with the right class, 
> >>the default convertor for the type and locale is used.
> > 
> > 
> > Wouldn't it be easier to just throw an exception if the correct type of
> > convertor couldn't be found. The fallback convertor probably wouldn't be
> > what the page author had in mind anyway. Regarding locale fallback,
> > shouldn't this be up to the convertors themselves as only they know if
> > they are locale sensitive. In the example above the DefaultDateConvertor
> > can handle any locale whereas the CustomDateConvertor should throw an
> > exception if there is no pattern for the locale or alternatively use a
> > default pattern.
> 
> Locale fallback is IMHO a necesity. For type I don't know, from the CSS 
> class analogy we should have it, but that is just an analogy and can be 
> missleading. Use cases are needed to decide. Leszek and Ralph you had 
> lots of opinions earlier in this thread, what do you say?
> 
> Depends also on how we distribute the decisions between the convertor 
> manager and the convertors. If we keep it close to CForms convertors, 
> type fallback probably becomes to complicated to be worthwhile.
> 
> Anyway, it is up to the implementor to decide how much effort it is worth.
> 
> >>Jonas, I still think that Bugzilla is a good place for your patches ;) 
> >>Please add the Apache license to all files also. And than you for your work.
> >  
> > I'll do so. I hope I won't add to much noise to the bug database.
> 
> Considering the quality of your work this far, the risk for noise seems 
> negligible :)

Thanks

> /Daniel

Cheers // Jonas


Mime
View raw message