Le lundi 12 novembre 2007 à 16:18 -0800, Nathan Bubna a écrit :
> ah.  ok, looked closer at the latest version.  looks good. thx. :)
> but now i have a new question...  why the expandSingletons stuff?  we
> don't expand them for $params.foo, why should we expand them for
> $params.foo.bar?
> 
> i'm assuming it's because request params automatically come as arrays
> when you iterate over the parameter map.   but i would like
> ValueParser to be more than just the base for ParameterTool, and it
> doesn't make sense for ValueParser.

Yes, I'll move the expansion stuff to ParameterTool, that's cleaner.
Also, we may reflect this expansion in all methods of the Map interface
for ParameterTool.

> > Btw, I hoped that by having ValueParser implement Map I'd see
> > ValueParser objects displayed like "{ key=value ... }" but I still see
> > the ugly [EMAIL PROTECTED], I'll dig
> > into this...
> 
> nothing special about implementing the Map interface when it comes to
> rendering.  Velocity simply renders stuff by calling toString().
> That's all that needs to change to change the display.

Extending the Map interface was also motivated to allow cool thinks like
$map.putAll($params)

I'll code us a nice toString().

> > > > The proposed implementation only returns a new ValueParser object
> > > > whenever subkeys are allowed and found.
> > >
> > > same could be done with a Sub.  granted, the implementation would have
> > > to be smarter (i.e. search for a potential matching subkeys first)
> > > than the last implementation for this, but there's no reason it can't
> > > be done.
> >
> > Sure, but it all boils down to the decision to keep the foo.int syntax
> > or not. If yes, then we definitely need a ValueParserSub object, if no,
> > then returning either a new ValueParser (when subkeys are found) or the
> > value in its integral type looks more natural to me.
> 
> yeah.  seems like just a question of whether we want the foo.int
> syntax or not.  since it doesn't seem to be your itch, don't worry
> about it.  if i get around to it, i'll do it.  if not, no big deal.
> 
> > My feeling here is that although foo.int is a cool syntax, it has too
> > many backwards ; especially, we should always return the integral type
> > (string, boolean or number) when available rather than a wrapper around
> > it to avoid nasty side effects.
> 
> c'mon, ValueParser is no less a wrapper than ValueParserSub would be.
> even a returning a simple HashMap would be returning a wrapper.   as
> long as we make the subkey business configurable (with it off when in
> deprecation support mode), and only return the
> ValueParser/ValueParserSub/HashMap for $params.foo when there are keys
> that start with "foo.", then i think we'll be fine.

You don't get my point. By construction, an application should usually
know when expecting a map or an integral type. Let's consider those
points separately:

 - maps: both implementations do wrap them. I guess both are valid but I
prefer the new one because I don't see why we should use two different
wrappers (ValueParser itself is already a wrapper around a map). Having
the wrapper extend Map respects the principle of least surprise for
template coders.

 - integral types: wrapping them is not a problem as long as they're
used directly for display (and it allows the foo.int syntax), but
whenever those values will be used as arguments to other tools or
objects methods you will encounter side effects (and don't even think of
trying to detect the appropriate conversion by reflection, it only works
for very simple cases). $list.add($params.foo) would add the
ValueParserSub to the list, not the integral type... and there are
plenty of examples like this one. In fact, specifying the type
(.int, .string, ...) becomes mandatory to get rid of the wrapper. Which
is rather cumbersome.

Re-introducing the foo.int syntax would be cool, of course, but why not
try to do it in the engine itself (so it is generalized to every value)?
It'd be cool to be able to do "$foo.int" on every value (once one have
checked that "int" is not a valid key for "foo").


   Claude

> <snip/>
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [EMAIL PROTECTED]
> For additional commands, e-mail: [EMAIL PROTECTED]
> 


---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]

Reply via email to