Hi,
sample patch code below...
Will Glass-Husain wrote:
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
Yup, this is what I mean. Since the #foreach has control of its body rendereing, it has the chance of inserting the chained context at this place. The chained context delegetes everything to the parent context. "#set"ing the loop variable to something non-null should set a flag within the chained context to disable it, thus affecting only the parent - thus preserving BC.
When the loop variable is non-null, #foreach could just use the original context; avoiding the additioanl chained context layer.
The chained context key=null inner class can be lazily instantiated when first needed (null encountered while iterating).
See sample patch code below!
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?)
It's in there now!
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
Sample code for a pach (untested, but good cahnces to work immediately!):
within the Foreach.render(), replaing the exiting while loop:
// no sync needed, since at most one instance per render() call!
NullHolderContext nullHolderContext = null; while( i.hasNext() )
{
context.put( counterName, new Integer(counter) );
Object value = i.next();
context.put(elementKey, value);
if( value == null )
{
if( nullHolderContext == null )
{
// lazy instantiation
nullHolderContext = new NullHolderContext(elementKey,
context);
}
node.jjtGetChild(3).render(nullHolderContext, writer);
}
else
{
node.jjtGetChild(3).render(context, writer);
}
counter++;
}Inner class for the Foreach directive:
/**
* A chained context as placeholder for a null valued loop variable.
*/
protected class NullHolderContext implements Context
{
private Context innerContext;
private String loopVariableKey = "";
private boolean active = true;
private NullHolderContext( String key, Context context )
{
innerContext = context;
if( key != null )
loopVariableKey = key;
}
public Object get( String key )
{
return ( active && loopVariableKey.equals(key) )
? null
: innerContext.get(key)
}
public Object put( String key, Object value )
{
if( loopVariableKey.equals(key) )
{
if( value == null )
{
active = true;
}
return innerContext.put( key, value )
}
}
public boolean containsKey( Object key )
{
return loopVariableKey.equals(key) || innerContext.containsKey(key);
}
public Object[] getKeys()
{
return innerContext.getKeys();
}
public Object remove(Object key)
{
if( loopVariableKey.equals(key) )
{
active = false;
}
return innerContext.remove(key);
}
}Cheers, Christoph
--------------------------------------------------------------------- To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
