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)
   <table>
     #foreach($user in $users)
       ...
     #end
   </table>
#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



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@velocity.apache.org
For additional commands, e-mail: dev-h...@velocity.apache.org

Reply via email to