Re: [RT] Attribute Rendering and CForms Convertors

[Daniel Fagerstrom]

Jonas Ekstedt wrote:
On Sun, 2004-11-14 at 22:57 +0100, Daniel Fagerstrom wrote:
 

<snip/>
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.
 

Ok.
<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.
 

Had not seen that, I just assumed that your Bugzilla entry was identical 
to the el.zip on your web site. That was obviously not true.

<snip/>
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.
 

Yes, I think we keep it as is, I have no important use cases that 
requires the convertor manager to handle all fallback.

I need to review your code a little bit more, then I'll commit the 
conversion block to the trunk (if no one protest), so that people can 
start to work on it. The code for using the convertor block in CForms 
has to wait a little bit longer as it is to early to make the CForms 
block dependent on the conversion block.

The next steps, IMO, would be to put copies of all the CForms convertors 
modified for the new interface, in the conversion block. And of course 
to iron out wrinkles in the interfaces. Then when everybody is happy 
about the state of the convertor block, we can refactor CForms so that 
it uses the convertors from the converrtor block instead, probably with 
some wrapping code so that we don't need to change any interfaces in 
CForms. We can also introduce it in e.g. JXTemplateGenerator.

WDYT?
/Daniel