Christoph,
Thanks for the detailed explanation of the alternative approaches. To sum up then, $foreach puts the iterating value into the current context. Since we can't assume that the current context will allow a value of null, this approach is not safe.
I like the idea of adding a chained context. It'd be nice if this could delegate to the current context for all non-null values. (Have to look into this). If the user does a #set in the middle of the #foreach, it's important the semantics of the original context be preserved. (For example, the parent context might be case-insensitive)
Can I post your explanation as a comment in bugzilla? (Or do you want to?)
WILL
----- Original Message ----- From: "Christoph Reck" <[EMAIL PROTECTED]>
To: "Velocity Developers List" <[EMAIL PROTECTED]>
Sent: Friday, December 17, 2004 2:01 PM
Subject: Re: null in foreach
Hi,
+1 to make a #foreach act properly when it encounters a null.
-1 (non binding) for the available patch. Reasoning is next:
The historical source of why velocity does not handle nulls: Originally vel started off with the posibility of using a simple Hashtable as the context - which does not allow nulls.
a) If we where allowed to set null into the context - no problem.
b) We could also wrap the user context into a chained one with only the #foreach var - which does allow setting it to null. -> This could raise some BC problem, since after the #foreach loop the original context may be restored, but the application wants to see the last value we had in the loop.
c) When setting null into the map value is not allowed, removing the key could yield the desired result. -> but beware... if the context was a chained context, the parent key could suddendly show - ugh. -> also beware that the parent context could be read-only and thus may not be altered...
d) The best solution is to have a pre-made key=null context at hand in the foreach directive instance to overlay it as a chained context every time a null is encountered.
Hope this helps to understand the whole problem.
I hope the coding of solution d) is easy as it could be made as an inner class to the #foreach directive and could be lazily instantiated upon the first use.
The solution d) is not possible for a #set directive, since it does not have a body and thus it is not possible to overlay a chained context. So the pittfalls of c) come to show...
Cheers, Christoph
Will Glass-Husain wrote:Hi,
Does anyone have any strong opinions about null in foreach? (specifically, reasons why we should not do this patch).
http://issues.apache.org/bugzilla/show_bug.cgi?id=27741
Note - I see this as a separate issue from the #set and #if handling of nulls. The problem with #foreach is that when looping through a collection containing a null, if a null is hit the iterator carries the old value rather than a null. This just doesn't make sense to me.
The provided patch (by Shinobu - thanks!) fixes it, but uses a config property to use old vs new behavior. I'd like to drop the config property and just make #for work as expected, e.g. return the current value of the collection (null or not).
Are there any compatibility issues with this?
WILL
--------------------------------------------------------------------- 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]
