Re: Velocity truth
Another try:
1) return false for a null object
2) return its value for a Boolean object, or the result of the
getAsBoolean() method if it exists.
3) If directive.if.emptycheck = false (true by default), stop here and
return true.
4) check for emptiness:
- return whether an array is empty.
- return whether isEmpty() is false (covers String and all Collection
classes).
- return whether length() is zero (covers CharSequence classes other
than String).
- returns whether size() is zero.
- return whether a Number *strictly* equals zero.
5) check for emptiness after explicit conversion methods:
- return whether the result of getAsString() is empty (and false for
a null result) if it exists.
- return whether the result of getAsNumber() *strictly* equals zero
(and false for a null result) if it exists.
About toString(), I agree that we could simply drop it along with its
configuration setting: we're talking about non-basic objects, which
don't have any size() or length() or isEmpty() method, and whose
toString() method could still return null or the empty string. Pretty
rare, I guess. And for such an object, the user may only want to check
whether it's null or not when he writes #if($foo).
We will also clearly state in the docs that checking for null can be
done with #if($foo == $null), for false with #if($foo == false), and for
empty string with #if("$!foo" == "").
Claude
On 06/02/2017 19:55, Michael Osipov wrote:
Am 2017-02-06 um 19:45 schrieb Nathan Bubna:
On Mon, Feb 6, 2017 at 10:32 AM, Michael Osipov
wrote:
Am 2017-02-06 um 19:23 schrieb Nathan Bubna:
On Mon, Feb 6, 2017 at 9:43 AM, Michael Osipov
wrote:
7) check object for a length() or size() method, if so return
whether it
returns 0, but I agree with Alex Fedotov that we could skip those
methods if we already took care for strings and collections.
What happened to array#length? You completely missed that out.
I would not drop #length() and #size(), you'd ultimately fail with
javax.naming.directory.Attribute#size() or
javax.naming.directory.Attributes#size().
There are likely more examples to have this.
i don't think it hurts to keep them, most users won't often get
that far,
i
think.
I did not say we should drop them, I said that they are crucial in some
situations. Dropping them would be wrong.
Yup. I was agreeing. :)
8) If directive.if.tostring.check = false, stop here and return true
(*)
9) check object for a toString() method, and return whether the
string
is non-null and non-empty
The 6th step won't be reached very often...
(*) the old configuration parameter was
directive.if.tostring.nullchec
k,
but for consistency with the new behavior regarding empty
strings, my
proposal is to rename it like this. I don't consider that backward
compatibility is important since all collections are handled
above in
the chain.
9) is somewhat confusing because #toString() is never null unless you
override it and return a custom string. Moreover, how will a
#toString()
guarantee you that an object is logically empty? It can't, see
javax.naming.directory.SearchResult#getAttributes().
At best, this would be false by default in 2.0.
...
I still back this check. Velocity is for templating, text output,
i.e. a
display language. Velocity-specific classes (including some
VelocityTools)
have had cause in the past to return null or empty strings
specifically
because of this check and that toString() is regularly central to
rendering
objects. While it cannot guarantee emptiness for every object out
there,
it
is a sensible check. The goal here is not perfection, but to catch
common
cases, and due to history, i believe this is a common one.
Again, there might be cases this is necessary though I cannot make
up one
from the top of my head. I am just saying that this should not be a
default
setting and people must know what they enable at the end.
My concern was backward compatibility, which, while not necessary, is
still
quite valuable. There are existing VelocityTools that rely on this,
after
all. Given all the ways to avoid getting to this check, i don't see the
compelling reason to toggle the default here. Though, as always, i will
defer to those doing actual work right now. :) Just chiming in with
my two
bits.
This is a new major release (!) for a for period of time, approaches
chang, so should software. We shall take the freedom and do the right
step forward. After all, people don't like surprises if the world
keeps revolving and you don't move with it.
-
To unsubscribe, e-mail: dev-unsubscr...@velocity.apache.org
For additional commands, e-mail: dev-h...@velocity.apache.org
-
To unsubscribe, e-mail: dev-unsubscr...@velocity.apache.org
For additional commands, e-mail: dev-h...@velocity.apache.org
Re: Velocity truth
Am 2017-02-06 um 19:45 schrieb Nathan Bubna: On Mon, Feb 6, 2017 at 10:32 AM, Michael Osipov wrote: Am 2017-02-06 um 19:23 schrieb Nathan Bubna: On Mon, Feb 6, 2017 at 9:43 AM, Michael Osipov wrote: 7) check object for a length() or size() method, if so return whether it returns 0, but I agree with Alex Fedotov that we could skip those methods if we already took care for strings and collections. What happened to array#length? You completely missed that out. I would not drop #length() and #size(), you'd ultimately fail with javax.naming.directory.Attribute#size() or javax.naming.directory.Attributes#size(). There are likely more examples to have this. i don't think it hurts to keep them, most users won't often get that far, i think. I did not say we should drop them, I said that they are crucial in some situations. Dropping them would be wrong. Yup. I was agreeing. :) 8) If directive.if.tostring.check = false, stop here and return true (*) 9) check object for a toString() method, and return whether the string is non-null and non-empty The 6th step won't be reached very often... (*) the old configuration parameter was directive.if.tostring.nullchec k, but for consistency with the new behavior regarding empty strings, my proposal is to rename it like this. I don't consider that backward compatibility is important since all collections are handled above in the chain. 9) is somewhat confusing because #toString() is never null unless you override it and return a custom string. Moreover, how will a #toString() guarantee you that an object is logically empty? It can't, see javax.naming.directory.SearchResult#getAttributes(). At best, this would be false by default in 2.0. ... I still back this check. Velocity is for templating, text output, i.e. a display language. Velocity-specific classes (including some VelocityTools) have had cause in the past to return null or empty strings specifically because of this check and that toString() is regularly central to rendering objects. While it cannot guarantee emptiness for every object out there, it is a sensible check. The goal here is not perfection, but to catch common cases, and due to history, i believe this is a common one. Again, there might be cases this is necessary though I cannot make up one from the top of my head. I am just saying that this should not be a default setting and people must know what they enable at the end. My concern was backward compatibility, which, while not necessary, is still quite valuable. There are existing VelocityTools that rely on this, after all. Given all the ways to avoid getting to this check, i don't see the compelling reason to toggle the default here. Though, as always, i will defer to those doing actual work right now. :) Just chiming in with my two bits. This is a new major release (!) for a for period of time, approaches chang, so should software. We shall take the freedom and do the right step forward. After all, people don't like surprises if the world keeps revolving and you don't move with it. - To unsubscribe, e-mail: dev-unsubscr...@velocity.apache.org For additional commands, e-mail: dev-h...@velocity.apache.org
Re: Velocity truth
On Mon, Feb 6, 2017 at 10:32 AM, Michael Osipov wrote: > Am 2017-02-06 um 19:23 schrieb Nathan Bubna: > >> On Mon, Feb 6, 2017 at 9:43 AM, Michael Osipov >> wrote: >> >>> 7) check object for a length() or size() method, if so return whether it >>> returns 0, but I agree with Alex Fedotov that we could skip those methods if we already took care for strings and collections. >>> What happened to array#length? You completely missed that out. >>> I would not drop #length() and #size(), you'd ultimately fail with >>> javax.naming.directory.Attribute#size() or >>> javax.naming.directory.Attributes#size(). >>> There are likely more examples to have this. >>> >> >> >> i don't think it hurts to keep them, most users won't often get that far, >> i >> think. >> > > I did not say we should drop them, I said that they are crucial in some > situations. Dropping them would be wrong. Yup. I was agreeing. :) > 8) If directive.if.tostring.check = false, stop here and return true (*) >> >>> 9) check object for a toString() method, and return whether the string is non-null and non-empty The 6th step won't be reached very often... (*) the old configuration parameter was directive.if.tostring.nullchec k, but for consistency with the new behavior regarding empty strings, my proposal is to rename it like this. I don't consider that backward compatibility is important since all collections are handled above in the chain. >>> 9) is somewhat confusing because #toString() is never null unless you >>> override it and return a custom string. Moreover, how will a #toString() >>> guarantee you that an object is logically empty? It can't, see >>> javax.naming.directory.SearchResult#getAttributes(). >>> >>> At best, this would be false by default in 2.0. >>> >> >> ... >> >> I still back this check. Velocity is for templating, text output, i.e. a >> display language. Velocity-specific classes (including some VelocityTools) >> have had cause in the past to return null or empty strings specifically >> because of this check and that toString() is regularly central to >> rendering >> objects. While it cannot guarantee emptiness for every object out there, >> it >> is a sensible check. The goal here is not perfection, but to catch common >> cases, and due to history, i believe this is a common one. >> > > Again, there might be cases this is necessary though I cannot make up one > from the top of my head. I am just saying that this should not be a default > setting and people must know what they enable at the end. My concern was backward compatibility, which, while not necessary, is still quite valuable. There are existing VelocityTools that rely on this, after all. Given all the ways to avoid getting to this check, i don't see the compelling reason to toggle the default here. Though, as always, i will defer to those doing actual work right now. :) Just chiming in with my two bits.
Re: Velocity truth
Am 2017-02-06 um 19:23 schrieb Nathan Bubna: On Mon, Feb 6, 2017 at 9:43 AM, Michael Osipov wrote: 7) check object for a length() or size() method, if so return whether it returns 0, but I agree with Alex Fedotov that we could skip those methods if we already took care for strings and collections. What happened to array#length? You completely missed that out. I would not drop #length() and #size(), you'd ultimately fail with javax.naming.directory.Attribute#size() or javax.naming.directory.Attributes#size(). There are likely more examples to have this. i don't think it hurts to keep them, most users won't often get that far, i think. I did not say we should drop them, I said that they are crucial in some situations. Dropping them would be wrong. 8) If directive.if.tostring.check = false, stop here and return true (*) 9) check object for a toString() method, and return whether the string is non-null and non-empty The 6th step won't be reached very often... (*) the old configuration parameter was directive.if.tostring.nullcheck, but for consistency with the new behavior regarding empty strings, my proposal is to rename it like this. I don't consider that backward compatibility is important since all collections are handled above in the chain. 9) is somewhat confusing because #toString() is never null unless you override it and return a custom string. Moreover, how will a #toString() guarantee you that an object is logically empty? It can't, see javax.naming.directory.SearchResult#getAttributes(). At best, this would be false by default in 2.0. ... I still back this check. Velocity is for templating, text output, i.e. a display language. Velocity-specific classes (including some VelocityTools) have had cause in the past to return null or empty strings specifically because of this check and that toString() is regularly central to rendering objects. While it cannot guarantee emptiness for every object out there, it is a sensible check. The goal here is not perfection, but to catch common cases, and due to history, i believe this is a common one. Again, there might be cases this is necessary though I cannot make up one from the top of my head. I am just saying that this should not be a default setting and people must know what they enable at the end. Michael - To unsubscribe, e-mail: dev-unsubscr...@velocity.apache.org For additional commands, e-mail: dev-h...@velocity.apache.org
Re: Velocity truth
On Mon, Feb 6, 2017 at 9:43 AM, Michael Osipov wrote: ... > 4) check for empty objects by class: >> - return whether the collection is empty for a Collection object >> >> 5) check object for an isEmpty() method, if so return whether it >> returned false >> > > Why explicitly check for collection if you do #isEmpty()? > I'd either add Map to the explicit check or drop collection altogether and > rely on #isEmpty() solely. makes sense to me. > 7) check object for a length() or size() method, if so return whether it >> returns 0, but I agree with Alex Fedotov that we could skip those >> methods if we already took care for strings and collections. >> > > What happened to array#length? You completely missed that out. > I would not drop #length() and #size(), you'd ultimately fail with > javax.naming.directory.Attribute#size() or > javax.naming.directory.Attributes#size(). > There are likely more examples to have this. i don't think it hurts to keep them, most users won't often get that far, i think. 8) If directive.if.tostring.check = false, stop here and return true (*) >> >> 9) check object for a toString() method, and return whether the string >> is non-null and non-empty >> >> The 6th step won't be reached very often... >> >> (*) the old configuration parameter was directive.if.tostring.nullcheck, >> but for consistency with the new behavior regarding empty strings, my >> proposal is to rename it like this. I don't consider that backward >> compatibility is important since all collections are handled above in >> the chain. >> > > 9) is somewhat confusing because #toString() is never null unless you > override it and return a custom string. Moreover, how will a #toString() > guarantee you that an object is logically empty? It can't, see > javax.naming.directory.SearchResult#getAttributes(). > > At best, this would be false by default in 2.0. ... I still back this check. Velocity is for templating, text output, i.e. a display language. Velocity-specific classes (including some VelocityTools) have had cause in the past to return null or empty strings specifically because of this check and that toString() is regularly central to rendering objects. While it cannot guarantee emptiness for every object out there, it is a sensible check. The goal here is not perfection, but to catch common cases, and due to history, i believe this is a common one.
Re: Velocity truth
Hi Claude, Am 2017-02-06 um 17:55 schrieb Claude Brisson: Hi Christopher. The spec has evolved quite a bit since then: the course I've taken is this one (and remarks are welcome): 4) check for empty objects by class: - return whether the collection is empty for a Collection object 5) check object for an isEmpty() method, if so return whether it returned false Why explicitly check for collection if you do #isEmpty()? I'd either add Map to the explicit check or drop collection altogether and rely on #isEmpty() solely. 7) check object for a length() or size() method, if so return whether it returns 0, but I agree with Alex Fedotov that we could skip those methods if we already took care for strings and collections. What happened to array#length? You completely missed that out. I would not drop #length() and #size(), you'd ultimately fail with javax.naming.directory.Attribute#size() or javax.naming.directory.Attributes#size(). There are likely more examples to have this. 8) If directive.if.tostring.check = false, stop here and return true (*) 9) check object for a toString() method, and return whether the string is non-null and non-empty The 6th step won't be reached very often... (*) the old configuration parameter was directive.if.tostring.nullcheck, but for consistency with the new behavior regarding empty strings, my proposal is to rename it like this. I don't consider that backward compatibility is important since all collections are handled above in the chain. 9) is somewhat confusing because #toString() is never null unless you override it and return a custom string. Moreover, how will a #toString() guarantee you that an object is logically empty? It can't, see javax.naming.directory.SearchResult#getAttributes(). At best, this would be false by default in 2.0. Michael On 06/02/2017 16:48, Christopher Schultz wrote: Claude, On 1/28/17 10:15 AM, Claude Brisson wrote: Here's what had been specified by Nathan at the time (order is meaningful, and falseness seems easier to specify than truth): $obj is null $obj is boolean false $obj returns false from getAsBoolean() (provided there is such a method) $obj is empty string (CharSequence w/length 0) $obj returns true from isEmpty() (provided there is such a method) $obj is array of length 0 $obj returns null from getAsString() (provided there is such a method) $obj returns empty string from getAsString() (provided there is such a method) $obj returns null from getAsNumber() (provided there is such a method) $obj returns 0 from length() or size() (provided there is such a method) $obj returns empty string from toString() (provided there is such a method) I *hate* that last one. A great use-case that ran us into OOMEs for a while until I figured out what was going on: 1. SELECT [fields] FROM table 2. Build ArrayList with e.g. User objects 3. Build a user list in HTML from a Velocity template like this: #if($users) #foreach($user in $users) ... #end #else No users found :() #end This gives me horrible performance and an OOME when the list gets too long, because the check for #if($users) truthiness converts the whole list collection into a String (which takes forever) which can be huge (which can cause OOME). I have now set the "directive.if.tostring.nullcheck=false" configuration property (and written a set of wrapper classes around Collection classes that throws an exception when toString is called, so things fail in development) to avoid this, but also taken to using this check instead: #if($users.size() > 0) But this gets me a warning about the "size" method not existing on a null object when the list is null. So I get junk in my logs when I do things the hacky-way and I get performance problems and OOMEs when I do things the "correct" way (at least, it looks totally correct). Regarding this spec: - I'm not sure about getAsString() ; toString() is usually the standard way of getting the String representation and should be enough. - I'm not convinced by the fact that zero should be true. I hear Nathan's point that for a display language, zero is as legitimate as any other number to be displayed. But it breaks the principle of least surprise, since each and every other language around, when not forbidding number towards boolean implicit conversion, consider zero as false. So I'd rather go with: $obj is null $obj is Boolean false $obj is Number zero (whatever Number variant) For floating point values, does this have to be *zero*, or just close enough to zero? $obj returns false from getAsBoolean() (provided there is such a method) $obj is empty string (CharSequence w/length 0) $obj returns true from isEmpty() (provided there is such a method) $obj is array of length 0 $obj returns null from getAsNumber() (provided there is such a method) $obj returns 0 from length() or size() (provided there is such a method) $obj returns empty string from toString() (provided there is such a metho
Re: Velocity truth
Hi Christopher. The spec has evolved quite a bit since then: the course I've taken is this one (and remarks are welcome): 1) return false for a null object 2) return its value for a Boolean object, or the result of the getAsBoolean() method if it exists. 3) If directive.if.emptycheck = false, stop here and return true. 4) check for empty objects by class: - return whether the number *strictly* equals zero for a Number object - return whether the string is empty for a CharSequence object - return whether the collection is empty for a Collection object 5) check object for an isEmpty() method, if so return whether it returned false 6) check object for explicit conversion methods: - return the result of getAsBoolean() (and false for a null result) if it exists - return whether the result of getAsNumber() *strictly* equals zero (and false for a null result) if it exists - return whether the result of getAsString() is empty (and false for a null result) if it exists 7) check object for a length() or size() method, if so return whether it returns 0, but I agree with Alex Fedotov that we could skip those methods if we already took care for strings and collections. 8) If directive.if.tostring.check = false, stop here and return true (*) 9) check object for a toString() method, and return whether the string is non-null and non-empty The 6th step won't be reached very often... (*) the old configuration parameter was directive.if.tostring.nullcheck, but for consistency with the new behavior regarding empty strings, my proposal is to rename it like this. I don't consider that backward compatibility is important since all collections are handled above in the chain. Claude On 06/02/2017 16:48, Christopher Schultz wrote: Claude, On 1/28/17 10:15 AM, Claude Brisson wrote: Here's what had been specified by Nathan at the time (order is meaningful, and falseness seems easier to specify than truth): $obj is null $obj is boolean false $obj returns false from getAsBoolean() (provided there is such a method) $obj is empty string (CharSequence w/length 0) $obj returns true from isEmpty() (provided there is such a method) $obj is array of length 0 $obj returns null from getAsString() (provided there is such a method) $obj returns empty string from getAsString() (provided there is such a method) $obj returns null from getAsNumber() (provided there is such a method) $obj returns 0 from length() or size() (provided there is such a method) $obj returns empty string from toString() (provided there is such a method) I *hate* that last one. A great use-case that ran us into OOMEs for a while until I figured out what was going on: 1. SELECT [fields] FROM table 2. Build ArrayList with e.g. User objects 3. Build a user list in HTML from a Velocity template like this: #if($users) #foreach($user in $users) ... #end #else No users found :() #end This gives me horrible performance and an OOME when the list gets too long, because the check for #if($users) truthiness converts the whole list collection into a String (which takes forever) which can be huge (which can cause OOME). I have now set the "directive.if.tostring.nullcheck=false" configuration property (and written a set of wrapper classes around Collection classes that throws an exception when toString is called, so things fail in development) to avoid this, but also taken to using this check instead: #if($users.size() > 0) But this gets me a warning about the "size" method not existing on a null object when the list is null. So I get junk in my logs when I do things the hacky-way and I get performance problems and OOMEs when I do things the "correct" way (at least, it looks totally correct). Regarding this spec: - I'm not sure about getAsString() ; toString() is usually the standard way of getting the String representation and should be enough. - I'm not convinced by the fact that zero should be true. I hear Nathan's point that for a display language, zero is as legitimate as any other number to be displayed. But it breaks the principle of least surprise, since each and every other language around, when not forbidding number towards boolean implicit conversion, consider zero as false. So I'd rather go with: $obj is null $obj is Boolean false $obj is Number zero (whatever Number variant) For floating point values, does this have to be *zero*, or just close enough to zero? $obj returns false from getAsBoolean() (provided there is such a method) $obj is empty string (CharSequence w/length 0) $obj returns true from isEmpty() (provided there is such a method) $obj is array of length 0 $obj returns null from getAsNumber() (provided there is such a method) $obj returns 0 from length() or size() (provided there is such a method) $obj returns empty string from toString() (provided there is such a method) Also, I noticed that Velocity weren't very consistent with literals. The only literal returning
Re: Velocity truth
On Mon, Feb 6, 2017 at 7:48 AM, Christopher Schultz < ch...@christopherschultz.net> wrote: > Claude, ... > > $obj returns false from getAsBoolean() (provided there is such a method) > > $obj is empty string (CharSequence w/length 0) > > $obj returns true from isEmpty() (provided there is such a method) > > $obj is array of length 0 > > $obj returns null from getAsNumber() (provided there is such a method) > > $obj returns 0 from length() or size() (provided there is such a method) > > $obj returns empty string from toString() (provided there is such a > method) > > > > Also, I noticed that Velocity weren't very consistent with literals. The > > only literal returning true is the 'true' literal. "foo" is false, > > whereas it should be consistent with $foo containing "foo". > > Can we maybe make an exception for Collections? Maybe for a Collection > (or array), we never call toString() on it? [].toString will always give > you garbage (which will be truthy) and Collection.toString() will also > likely give you garbage and it will also always be truthy unless it's > (a) null or (b) the Collection implements toString in a surprising way > relative to java.util Collections. > ... Yeah, one of the crucial ideas here is that Velocity would stop checking as soon as it found one of these. So no Array, CharSequence, or Collection would ever make it down the lookup chain to toString(). The goal is to be more type/function aware specifically to avoid getting down to the toString(), but still leaving that toString() check for better backward compatibility.
Re: Velocity truth
I would try to something similar to Javascript convention https://javascriptweblog.wordpress.com/2011/02/07/truth-equality-and-javascript/ substituting CharSequence instead of String (so that other empty String like objects i.e. StringBuilder would evaluate as false) and also treating empty arrays and collections as "false". Not sure if Duck typing for methods like isEmpty(), length() or size() is really necessary, could just support BooleanSupplier interface for custom conversions. All other cases would be covered by collections and arrays. Alex On Mon, Feb 6, 2017 at 10:48 AM, Christopher Schultz < ch...@christopherschultz.net> wrote: > Claude, > > On 1/28/17 10:15 AM, Claude Brisson wrote: > > Here's what had been specified by Nathan at the time (order is > > meaningful, and falseness seems easier to specify than truth): > > > > $obj is null > > $obj is boolean false > > $obj returns false from getAsBoolean() (provided there is such a method) > > $obj is empty string (CharSequence w/length 0) > > $obj returns true from isEmpty() (provided there is such a method) > > $obj is array of length 0 > > $obj returns null from getAsString() (provided there is such a method) > > $obj returns empty string from getAsString() (provided there is such a > > method) > > $obj returns null from getAsNumber() (provided there is such a method) > > $obj returns 0 from length() or size() (provided there is such a method) > > $obj returns empty string from toString() (provided there is such a > method) > > I *hate* that last one. A great use-case that ran us into OOMEs for a > while until I figured out what was going on: > > 1. SELECT [fields] FROM table > 2. Build ArrayList with e.g. User objects > 3. Build a user list in HTML from a Velocity template like this: > > #if($users) > > #foreach($user in $users) > ... > #end > > #else > No users found :() > #end > > This gives me horrible performance and an OOME when the list gets too > long, because the check for #if($users) truthiness converts the whole > list collection into a String (which takes forever) which can be huge > (which can cause OOME). > > I have now set the "directive.if.tostring.nullcheck=false" configuration > property (and written a set of wrapper classes around Collection classes > that throws an exception when toString is called, so things fail in > development) to avoid this, but also taken to using this check instead: > > #if($users.size() > 0) > > But this gets me a warning about the "size" method not existing on a > null object when the list is null. So I get junk in my logs when I do > things the hacky-way and I get performance problems and OOMEs when I do > things the "correct" way (at least, it looks totally correct). > > > Regarding this spec: > > - I'm not sure about getAsString() ; toString() is usually the standard > > way of getting the String representation and should be enough. > > - I'm not convinced by the fact that zero should be true. I hear > > Nathan's point that for a display language, zero is as legitimate as any > > other number to be displayed. But it breaks the principle of least > > surprise, since each and every other language around, when not > > forbidding number towards boolean implicit conversion, consider zero as > > false. > > > > So I'd rather go with: > > > > $obj is null > > $obj is Boolean false > > $obj is Number zero (whatever Number variant) > > For floating point values, does this have to be *zero*, or just close > enough to zero? > > > $obj returns false from getAsBoolean() (provided there is such a method) > > $obj is empty string (CharSequence w/length 0) > > $obj returns true from isEmpty() (provided there is such a method) > > $obj is array of length 0 > > $obj returns null from getAsNumber() (provided there is such a method) > > $obj returns 0 from length() or size() (provided there is such a method) > > $obj returns empty string from toString() (provided there is such a > method) > > > > Also, I noticed that Velocity weren't very consistent with literals. The > > only literal returning true is the 'true' literal. "foo" is false, > > whereas it should be consistent with $foo containing "foo". > > Can we maybe make an exception for Collections? Maybe for a Collection > (or array), we never call toString() on it? [].toString will always give > you garbage (which will be truthy) and Collection.toString() will also > likely give you garbage and it will also always be truthy unless it's > (a) null or (b) the Collection implements toString in a surprising way > relative to java.util Collections. > > -chris > >
Re: Velocity truth
Claude, On 1/28/17 10:15 AM, Claude Brisson wrote: > Here's what had been specified by Nathan at the time (order is > meaningful, and falseness seems easier to specify than truth): > > $obj is null > $obj is boolean false > $obj returns false from getAsBoolean() (provided there is such a method) > $obj is empty string (CharSequence w/length 0) > $obj returns true from isEmpty() (provided there is such a method) > $obj is array of length 0 > $obj returns null from getAsString() (provided there is such a method) > $obj returns empty string from getAsString() (provided there is such a > method) > $obj returns null from getAsNumber() (provided there is such a method) > $obj returns 0 from length() or size() (provided there is such a method) > $obj returns empty string from toString() (provided there is such a method) I *hate* that last one. A great use-case that ran us into OOMEs for a while until I figured out what was going on: 1. SELECT [fields] FROM table 2. Build ArrayList with e.g. User objects 3. Build a user list in HTML from a Velocity template like this: #if($users) #foreach($user in $users) ... #end #else No users found :() #end This gives me horrible performance and an OOME when the list gets too long, because the check for #if($users) truthiness converts the whole list collection into a String (which takes forever) which can be huge (which can cause OOME). I have now set the "directive.if.tostring.nullcheck=false" configuration property (and written a set of wrapper classes around Collection classes that throws an exception when toString is called, so things fail in development) to avoid this, but also taken to using this check instead: #if($users.size() > 0) But this gets me a warning about the "size" method not existing on a null object when the list is null. So I get junk in my logs when I do things the hacky-way and I get performance problems and OOMEs when I do things the "correct" way (at least, it looks totally correct). > Regarding this spec: > - I'm not sure about getAsString() ; toString() is usually the standard > way of getting the String representation and should be enough. > - I'm not convinced by the fact that zero should be true. I hear > Nathan's point that for a display language, zero is as legitimate as any > other number to be displayed. But it breaks the principle of least > surprise, since each and every other language around, when not > forbidding number towards boolean implicit conversion, consider zero as > false. > > So I'd rather go with: > > $obj is null > $obj is Boolean false > $obj is Number zero (whatever Number variant) For floating point values, does this have to be *zero*, or just close enough to zero? > $obj returns false from getAsBoolean() (provided there is such a method) > $obj is empty string (CharSequence w/length 0) > $obj returns true from isEmpty() (provided there is such a method) > $obj is array of length 0 > $obj returns null from getAsNumber() (provided there is such a method) > $obj returns 0 from length() or size() (provided there is such a method) > $obj returns empty string from toString() (provided there is such a method) > > Also, I noticed that Velocity weren't very consistent with literals. The > only literal returning true is the 'true' literal. "foo" is false, > whereas it should be consistent with $foo containing "foo". Can we maybe make an exception for Collections? Maybe for a Collection (or array), we never call toString() on it? [].toString will always give you garbage (which will be truthy) and Collection.toString() will also likely give you garbage and it will also always be truthy unless it's (a) null or (b) the Collection implements toString in a surprising way relative to java.util Collections. -chris signature.asc Description: OpenPGP digital signature
Re: [VOTE] Michael Osipov as committer
Nathan, Hey! I didn't know I was on the PMC. :) Vote below. On 1/27/17 1:05 PM, Nathan Bubna wrote: > Hey folks, > > Michael Osipov has been rigorously and vigorously reviewing the progress of > Engine 2.0, with commits specific and knowledgeable to be nigh > indistinguishable from code itself. He's already an ASF committer (with > Maven at least, maybe Jersey too?). In any case, he knows our ways and is > actively contributing to Velocity. When a contributor starts keeping a > committer busy (hang in there, Claude), we reward them with commit access > so they can do it themselves. :) > > What say ye? > > > [X] +1 Sounds good. > [ ] +/- 0 Not sure, because... > [ ] -1 No. Because... +1 -chris signature.asc Description: OpenPGP digital signature
