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 <micha...@apache.org> wrote:

Am 2017-02-06 um 19:23 schrieb Nathan Bubna:

On Mon, Feb 6, 2017 at 9:43 AM, Michael Osipov <micha...@apache.org>
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

Reply via email to