Re: Formatter for configurable MVC [long]

[Christoph Reck <Ch...@dlr.de>]

Returned from the turbine list...

Fedor Karpelevitch wrote:
> 
> >  The desing pattern seems to be cristalizing with two reviewers.
> >  Could the Velocity contributors give votes whe we've finalized
> >  this proposal?
> 
> Would be nice!
> BTW, what the status of formatters should be? Probably a plugin for
> velocity? Or integral part? Or standalone service? Part of Turbine?

Formatting is an issue for the view part of MVC, therefore the support 
could/should be integral part of velocity, as the interface
definitions and some basic formatters. Other formatters are atteched
via configuration.

Fedor, please do not cross post to the turbine list.

> 
> >  >[snip]
> >  > I think business logic should be responsible for security rather than
> >  > formatter. Can you give an example of how you see the security 
> >  > handling in formatters?
> >
> >  The formatter can look into the context to determine if an edit flag
> >  is set
> I do not think context is the right place for this flag because different
> controls on the same page (and so with the same context) may need different
> readOnly flag. Rather than passing this flag in the context it makes much
> more sense to me that the object being passed ( for instance let's call it
> EditBox ) should have set/getReadOnly() methods. When controller creates the
> object it sets readOnly flag properly. When formatter renders the object it
> calls getReadOnly to determine what mode should it use.

The edit flag is just an example. Change the example to look into 
DB and UserRole...

The BL should be separated from the context and from the security
(keep 'em simple), therefore the formatter should consult a third
(and central) instance if restrictions should apply. Its up to the
implementation where it will look for it (context, DB, config-file,
etc.).

> 
> >  and the user has rights to edit the desired object and then
> >  show the object using an edit template.
> 
> again. I think BL should look at permissions and set the readOnly property
> accordingly.
> 
> What I think passing Context to formatter can be rally useful for is to be
> able to format Object differently depending on useragent and/or user
> preferences. for example if you set readOnly property of TextBox to true
> Formatter will output it as "<input type=text READONLY..." for IE5 &
> Mozilla, but will make it Plain text (or use javascript) for Netscape 4.x
> which does not support "READONLY".

yet another sensible use for the context...

> 
> > Otherwise, uses the display
> >  template (a guest user might get an empty field). The security
> >  configuration resides elsewhere (config file or DB).
> agree
> 
> >  > >  >[snip]
> >  > >  > What do you think is the right search order:
> >  > >  >
> >  > >  > formatter - HTMLable - default
> >  > >
> >  > >  This way you can override and customize the hard-coded HTMLable
> >  > >  interface. More below.
> >  >
> >  > As I wrote before, I now think that we only need:
> >  >
> >  > formatter (ClassFormatter) - default
> >  >
> >  > the rest seems to be redundunt.
> >
> >  As initially stated, the object could be allowed to implement
> >  the Formatter interface (was HTMLable). The configuration
> >  defines classes with ConfigurationFormatter interface
> >  (which takes the object as an additional parameter).
> 
> Formatter and HTMLable are different. Formatter formats another object,
> HTMLable formats itself. I do not really see any need for HTMLable when we
> have formatters. They are much more flexible. In fact you can easily 

To clear the misunderstanding: there are two flavours of formatters
one that formats itself and one that formats other objects:
1. The first one could be a wrapper class instantiated by the controller
to contain the BL that it will format (keeping the model separate
from the view - but an implementation could implement the interface
directly on the BL). 
2. The second one is the only one that will work when specified within 
the configuration.
Therefore I called the two interfaces Formatter and ConfigurationFormatter.


> achieve HTMLable fuctionality using formatters:
> 
> <formatter-mapping>
>  <formatter-name>HTMLableFormatter</formatter-name>
>  <class-name>package.HTMLable</class-name>
> </formatter-mapping>
> 
> ---
> HTMLableFormatter.format(Object obj, Context ctx)
> {
>  return ((HTMLable)obj).toHTML(ctx);
> }
> ---

The object should not be required to format itself (keep model
and view separate!). But might be required by an implementation
to avoid the drawback stated below.

> 
> and this will be subject to configuration....
> 
> >  Note that the code snippets pass the Context and contextPath
> >  parameters to the format method to allow better control of what
> >  may be displayed - normally these parameters are ignored.
> 
> I do not think we need to pass contextPath to the formatter. BL should
> handle any changes based on CP. Formatter may run in different enviroments
> and should not care where it's running, rather than that it shouold be
> providied with information it needs to render the object.

It does currently the way WM works. I believe it makes sense to
allow configuring the instance in the Context (and not only the 
class name). Therefore $price (or the date within $session.getTimeout() )
could be configurable with its own formatter. The variable access
name is what i'm calling contextPath.

> 
> >  > > > case all Floats will be formatted as money, even if it was 
> >[snip]
> >  > I actually think now that this problem should rather be solved within
> the
> >  > ClassFormatter concept. To get a specific formatting for money you just do:
> >
> >  yes this is where the Formatter iterface comes into play (if the
> >  object wants to know about the context it is being used within).
> 
> I think it should still use classes to distinguish this...

The controlling application should allow a wrapper on the BL to
do the formatting.

> 
> >  Otherwise, it just implements its own toString()!
> >
> >  Note that the object still could be packed (via code) within a
> >  ConfigurationFormatter/PatternFormatter to delegate the formatting
> >  to this other class (very practical, so that the objects itself
> >  do not need to know anything about formatting - no new classes
> >  to be written!)
> 
> That's probably just implementayion details...

But an important one, which should be made integral part of velocity.

> 
> >  The drawback of this approach is that a object retrieved via
> >  a method could be of an original java.lang.* type knowing nothing
> >  of a propietary format.
> 
> I believe in this case you simply need to create your own class.

Keep BL separate from the view... allow contextPath configuration...

> 
> > This is solvable through the contextPath approach.
> 
> -1
> 
> >  >
> >  > public class Money extends Float{} // it can actulaay be doing smth
> >  >                                       money-specific like validation...
> >  >
> >  > and then you simply define a specific formatter for Money which does
> not
> >  > affect the way Float is formatted.
> >  >
> >
> >  Ugh! java.lang.* classes are final and cannot be extended.
> 
> mestupid.
> 
> what about using HasA in this case?
> 
> class Money{
>  private Float val;
>  public Float getValue(){}
> }

well again requires a new class to be written... It would be
nice to have a generic configurable approach that does not
require writing new classes - voila - the contextPath approach!

> 
> >  Anyway it should be:
> >    public class Money implements Formatter
> 
> -1
> I do not think BL should know nothing and never deal directly with
> formatters because they may change in different configurations.

This is what i've been stating over and over above (apart from the
drawback mentioned with a possible fix using either custom BL
implmenting Formatter). If your BL differentiates each object type
via its own class (e.g. Money.class above), then its wonderfully
configurable w/o contextPath approach and then we have a perfect 
world ;-) But it does not consider that existing BL classes might
not be that perfect...

> 
> >  > >  such that the whole option chain is then:
> >  > >    NamedFormatter - ConfigurationFormatter - Formatter - default -
> >  > toString()
> >  >
> >  > As I said I do  not like the idea of NamedFormatter. I believe it
> really
> >  > defeats the purpose of flexibility and anything it can accomplish can
> be
> >  > easily accomplished by ClassFormatter.
> >
> >  OK, leave this construct away (I agree it complicates things a bit,
> >  that can be solved otherwise).
> >
> >  >
> >  > I am not sure I understand the difference between Formatter and
> >  > ConfigurationFormatter.
> >
> >  The Formatter is an interface of the implementing object.
> >  The ConfigurationFormatter takes the object instance as parameter.
> 
> I believe ConfigurationFormatter is the only Formatter so it does not make
> sense to crate an interface and class separately. Why not just make
> Formatter the class which does all the work? This way Velocity will have
> less work to guess whether to pass context or not - it will pass it always.

No. the controller might already impose formatting on the obejcts...

> 
> >  > I think we do not need Default - toString() is a good shot for that. If you
> >  > really want a "default" formatter you simply define it for java.lang.Object,
> >  > no special status needed.
> >
> >  Would need reflection to step up the inherited classes, and would impact
> >  performance.
> 
> I think it will work automatically anyway. I do not think we need to take
> any additional steps to implement it and it looks like a nice feature (i do
> not like chained if - else - if -else - ...)

This is your personal opinion...

> 
> >
> >  By implementing a default formatter (defaults to DefaultFormatter
> >  which then uses toString) it is possible to remap the formatting pattern
> >  to a context sensistive pattern as WM is approaching.
> 
> I do not think anyone will ever want to use anything else but toString
> _by_default_

The default could be changed to be a contextPath evaluating formatter.

> 
> >  >[snip]
> >  > I think Formatter is created outside of request context, during init.
> >
> >  I was thinking of a class with static methods. But this may conflict
> >  with several applications within one VM. So its right to use the
> >  intialization phase of a specific formatter instance.
> 
> Right. We probably also need FormatterFactory.

not necesary... 

> 
> >  > So there will be no Context.
> >
> >  See top of mail. The Context parameter is the same as for the
> >  template, providing additional input to the formatter.
> 
> It will be there when you call format() but not at init time.

Yes.

> 
> >  > I think its constructor should not take any
> >  > parameters - it should pick up any addl info from config file.
> >  >
> >  > config file should look smth like:
> >  > ----
> >  > format=java.lang.Date,com.company.DateFormatter
> >  > format=com.company.Money,com.company.MoneyFornmatter
> >  >
> >  > formatter.DateForatter.dateFormat=MM/DD/YYYY
> >  > ----
> >
> >  Yes this seems to be the right INI approach for additional parameters :.(
> 
> I've provided XML version as well

Currently up to others to decide...

> 
> >  > one formatter can be used to hanle more than one class by overloading
> >  > format() method:
> >
> >  No. Would require reflection to find the right method. The
> >  format() method should just take an object (the Interface defines
> >  it so). Within the method itself it can check via instanceOf
> >  what was really passed.
> 
> would not it work just fine if I define format(Money amt) besides
> format(Object obj)?

No, because the invocation signature at compile time specifies
the method that will be used. So to avoid reflection, it is
only sensible to use a series of if-else if...

> 
> >  > ---
> >  > format=com.company.Dollar,com.company.MoneyFormatter
> >  > format=com.company.Euro,com.company.MoneyFormatter
> >  >
> >  > formatter.Moneyformatter.euroformat = "%1%<Euro>"
> >  > formatter.Moneyformatter.dollarformat = "$%1%"
> >  > ---
> >
> >  Fine. Works with 'public String format(Object value)' just the same.
> 
> Right, but I do not like ifelseifelseif (Show me who does).

Either a loop over reflection returned Methods or ifelseifelseif...

> 
> >  > Do we need FormatterService?
> >
> >  Yes, it seems to be the right approach for singletons, which need
> >  initialization once (vs. class with static methods).
> >
> >  >
> >  > Probably configuration should also handle defining formatters depending on
> >  > context path or even URL mask (I do not like putting it into Formatter
> >  > itself because you won't be able to configure it. Besides that Formatter can
> >  > be used somewhere where "URL" is not applicable at all)
> >
> >  I don't think that an object shall be formatted differently depending
> >  on the URL. The contextPath should be used instead. This is why I
> >  placed the comment into the DefaultFormatter stating that the contextPath
> >  should be looked up in the configuration for its context formatter.
> 
> contexPath may not be applicable either.

see usage above. The controller filling in the context with model
data specifies the contextPath to use, the confugration can be seen
as part of the controller. Page desingers only use what the controller
provided.

> 
> >  Back to the INI configuration: to define different formatter
> >  flavours using the convenience class, it will be necesary to
> >  provide at least one parameter to the constructor. See
> >  the TemplateFormatter configuration in the next mail in this
> >  thread.
> 
> Do you want to create one instance for each mapping? I believe one instance
> is enough and no parameters for constructor are needed (or maybe config
> data?). All formatter needs to know should be in <init-param>

I'm thinking of a generic formater used with multiple patterns
one for each instance. Each of these instances are then assinged to
a different class name (or contextPath).

> 
> >  [To the mail commenting the WM approcah in this thread]
> >  The WM approach is a feasible enhancement. If velocity is to
> >  be a follow-up of WM it should take over their template approach.
> 
> Do not think that Velocity should repeat WebMacro mistakes ;-)

Who judges if something is a mistake...

> 
> >  How shall we proceed? Who shall do the initial patch recomendation
> >  to Velocity?
> 
> I'll see when I'll be able to bite into the code. We yet need to decide what
> should be formatters' position in between of Velocity and Turbine
> 
> fedor.

I'll not be online next week.

Cheers,
Christoph