Le lundi 12 novembre 2007 à 10:53 -0800, Nathan Bubna a écrit :
> it's not the name so much as how it's used. it looks to me like once
> it searches for subkey "foo", it won't even try to find subkey "bar"
> or "woogie" or whatever. either that's wrong or i'm missing
> something?
The "hasSubkeys" boolean is here so that we don't search for subkeys
twice if we already know there isn't any (so that the overhead when
looking for inexisting keys, in templates that don't use subkeys, is
reduced to only one indeOf('.') in all key names). The very first
implementation was buggy, it may be the one you read.
> > > i'm also curious about what the advantages are of this approach (as
> > > opposed to a ValueParserSub). it's been a while since i've thought
> > > through the implementation options for this.
> >
> > Things have changed since then: the engine is now quite efficient in
> > automatic types conversions so the "foo.int" syntax is much less
> > necessary than "foo.bar", and you always have "foo.getInt()".
>
> huh? foo.int is the same as foo.getInt(). you can't have the latter
> if the former doesn't work. did you mean getInt('foo')?
Yes, sorry.
> also, the
> sub could handle both foo.bar and foo.int, and the engine can only do
> a small fraction of the conversions the ValueParser is capable of.
> so, while i don't see anything wrong with the current approach (apart
> from the hasSubKey thing), i still suspect a Sub has more potential,
> since we have complete control of the API.
>
> > But above all, having the generic getter return something else (that is,
> > the ValueParserSub) than the underlying basic type leads - at least in
> > my experience - to several side effects (like a jdbc driver being unable
> > to handle the ValueParserSub class by calling toString on it).
>
> not sure how returning ValueParserSub is that different from returning
> ValueParser.
We only return a ValueParser for get("foo") when there are "foo.bar"
keys, and keep returning the integral type otherwise. Since we expect
the same fonctionnalities for $params and $params.foo, the recursive
approach looked simpler to me.
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...
> > 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.
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.
Claude
> >
> >
> > Claude
> >
> >
> > > On Nov 9, 2007 7:15 AM, Claude Brisson <[EMAIL PROTECTED]> wrote:
> > > > ValueParser now has protected get/setAllowSubkeys() boolean methods.
> > > >
> > > > The default value of allowSubkey shoud be the value of deprecatedMode,
> > > > but I'm not really sure of how this should be done. But I'm sure Nathan
> > > > will be of some help here.
> > > >
> > > > Also, since ValueParser is used internally by the Tool.configure method,
> > > > it may be safer to move all this new stuff in ParameterParser only.
> > > >
> > > > Claude
> > > >
> > > >
> > > > Le vendredi 09 novembre 2007 à 14:49 +0000, [EMAIL PROTECTED] a
> > > > écrit :
> > > >
> > > > > Author: cbrisson
> > > > > Date: Fri Nov 9 06:49:36 2007
> > > > > New Revision: 593549
> > > > >
> > > > > URL: http://svn.apache.org/viewvc?rev=593549&view=rev
> > > > > Log:
> > > > > sub keys getter (not yet tested but doesnt break anything at least)
> > > > >
> > > > > Modified:
> > > > >
> > > > > velocity/tools/branches/2.x/src/main/java/org/apache/velocity/tools/generic/ValueParser.java
> > > > >
> > > > > velocity/tools/branches/2.x/src/main/java/org/apache/velocity/tools/view/ParameterTool.java
> > > > >
> > > > > Modified:
> > > > > velocity/tools/branches/2.x/src/main/java/org/apache/velocity/tools/generic/ValueParser.java
> > > > > URL:
> > > > > http://svn.apache.org/viewvc/velocity/tools/branches/2.x/src/main/java/org/apache/velocity/tools/generic/ValueParser.java?rev=593549&r1=593548&r2=593549&view=diff
> > > > > ==============================================================================
> > > > > ---
> > > > > velocity/tools/branches/2.x/src/main/java/org/apache/velocity/tools/generic/ValueParser.java
> > > > > (original)
> > > > > +++
> > > > > velocity/tools/branches/2.x/src/main/java/org/apache/velocity/tools/generic/ValueParser.java
> > > > > Fri Nov 9 06:49:36 2007
> > > > > @@ -21,6 +21,9 @@
> > > > >
> > > > > import java.util.Map;
> > > > > import java.util.Locale;
> > > > > +import java.util.Set;
> > > > > +import java.util.HashMap;
> > > > > +
> > > > > import org.apache.velocity.tools.config.DefaultKey;
> > > > >
> > > > > /**
> > > > > @@ -40,6 +43,14 @@
> > > > > {
> > > > > private Map source = null;
> > > > >
> > > > > + private boolean allowSubkeys = true; /* default to whatever,
> > > > > should be overridden by depreprecatedMode default value anyway */
> > > > > +
> > > > > + /* when using subkeys, cache at least the presence of any subkey,
> > > > > + so that the rendering of templates not using subkeys will only
> > > > > + look once for subkeys
> > > > > + */
> > > > > + private boolean hasSubkeys = true;
> > > > > +
> > > > > public ValueParser() {}
> > > > >
> > > > > public ValueParser(Map source)
> > > > > @@ -98,7 +109,11 @@
> > > > > {
> > > > > return null;
> > > > > }
> > > > > - return getSource().get(key);
> > > > > + Object value = getSource().get(key);
> > > > > + if (value == null && getAllowSubkeys()) {
> > > > > + value = getSubkey(key);
> > > > > + }
> > > > > + return value;
> > > > > }
> > > > >
> > > > > public Object[] getValues(String key)
> > > > > @@ -359,4 +374,29 @@
> > > > > return toLocales(getValues(key));
> > > > > }
> > > > >
> > > > > + protected boolean getAllowSubkeys() {
> > > > > + return allowSubkeys;
> > > > > + }
> > > > > +
> > > > > + protected void setAllowSubkeys(boolean allow) {
> > > > > + allowSubkeys = allow;
> > > > > + }
> > > > > +
> > > > > + protected Object getSubkey(String subkey) {
> > > > > + if (!hasSubkeys || subkey == null || subkey.length() == 0) {
> > > > > + return null;
> > > > > + }
> > > > > + Map<String,Object> values = null;
> > > > > + subkey = subkey.concat(".");
> > > > > + for(Map.Entry<String,Object>
> > > > > entry:(Set<Map.Entry>)getSource().entrySet()) {
> > > > > + if(entry.getKey().startsWith(subkey)) {
> > > > > + if(values == null) {
> > > > > + values = new HashMap<String,Object>();
> > > > > + }
> > > > > +
> > > > > values.put(entry.getKey().substring(subkey.length()),entry.getValue());
> > > > > + }
> > > > > + }
> > > > > + hasSubkeys = (values == null);
> > > > > + return values;
> > > > > + }
> > > > > }
> > > > >
> > > > > Modified:
> > > > > velocity/tools/branches/2.x/src/main/java/org/apache/velocity/tools/view/ParameterTool.java
> > > > > URL:
> > > > > http://svn.apache.org/viewvc/velocity/tools/branches/2.x/src/main/java/org/apache/velocity/tools/view/ParameterTool.java?rev=593549&r1=593548&r2=593549&view=diff
> > > > > ==============================================================================
> > > > > ---
> > > > > velocity/tools/branches/2.x/src/main/java/org/apache/velocity/tools/view/ParameterTool.java
> > > > > (original)
> > > > > +++
> > > > > velocity/tools/branches/2.x/src/main/java/org/apache/velocity/tools/view/ParameterTool.java
> > > > > Fri Nov 9 06:49:36 2007
> > > > > @@ -109,7 +109,11 @@
> > > > > */
> > > > > public Object getValue(String key)
> > > > > {
> > > > > - return getRequest().getParameter(key);
> > > > > + Object value = getRequest().getParameter(key);
> > > > > + if(value == null && getAllowSubkeys()) {
> > > > > + value = getSubkey(key);
> > > > > + }
> > > > > + return value;
> > > > > }
> > > > >
> > > > >
> > > > >
> > > > >
> > > >
> > > >
> > > > ---------------------------------------------------------------------
> > > > 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]
> > >
> >
> >
> > ---------------------------------------------------------------------
> > 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]
>
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
