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.

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

Reply via email to