Hi Adrian

I don't mind the change and I had considered doing it myself at one point. The reason I didn't was because I figured it was much more likely that someone would iterate over a list of nodes rather than directly access it so this change just means that the list is iterated over twice. I mean why would someone want to access a node at point x in the context of an xml document? You typically either want the first child or you want the whole list. Either way I don't mind but I just wanted to mention that I did think of doing this and chose not to.

Regards
Scott

HotWax Media
http://www.hotwaxmedia.com

On 3/05/2009, at 11:36 AM, adri...@apache.org wrote:

Author: adrianc
Date: Sat May  2 22:37:08 2009
New Revision: 770990

URL: http://svn.apache.org/viewvc?rev=770990&view=rev
Log:
Small change to Scott's UEL improvement - convert NodeList to a java.util.List so that it can be treated like a regular List.

Modified:
ofbiz/trunk/framework/base/src/org/ofbiz/base/util/string/ NodeELResolver.java ofbiz/trunk/framework/minilang/src/org/ofbiz/minilang/method/ envops/Iterate.java

Modified: ofbiz/trunk/framework/base/src/org/ofbiz/base/util/string/ NodeELResolver.java
URL: 
http://svn.apache.org/viewvc/ofbiz/trunk/framework/base/src/org/ofbiz/base/util/string/NodeELResolver.java?rev=770990&r1=770989&r2=770990&view=diff
= = = = = = = = ====================================================================== --- ofbiz/trunk/framework/base/src/org/ofbiz/base/util/string/ NodeELResolver.java (original) +++ ofbiz/trunk/framework/base/src/org/ofbiz/base/util/string/ NodeELResolver.java Sat May 2 22:37:08 2009
@@ -20,6 +20,7 @@

import java.beans.FeatureDescriptor;
import java.util.Iterator;
+import java.util.List;

import javax.el.CompositeELResolver;
import javax.el.ELContext;
@@ -31,6 +32,8 @@
import javax.xml.xpath.XPathExpressionException;
import javax.xml.xpath.XPathFactory;

+import javolution.util.FastList;
+
import org.apache.xerces.dom.NodeImpl;
import org.ofbiz.base.util.Debug;
import org.ofbiz.base.util.cache.UtilCache;
@@ -100,13 +103,16 @@
                } else if (nodeList.getLength() == 1) {
                    result = nodeList.item(0);
                } else {
-                    result = nodeList;
+                    List<Node> newList = FastList.newInstance();
+                    for (int i = 0; i < nodeList.getLength(); i++) {
+                        newList.add(nodeList.item(i));
+                    }
+                    result = newList;
                }
                context.setPropertyResolved(true);
            } catch (XPathExpressionException e) {
Debug.logError("An error occurred during XPath expression evaluation, error was: " + e, module);
            }
-
        }
        return result;
    }

Modified: ofbiz/trunk/framework/minilang/src/org/ofbiz/minilang/ method/envops/Iterate.java
URL: 
http://svn.apache.org/viewvc/ofbiz/trunk/framework/minilang/src/org/ofbiz/minilang/method/envops/Iterate.java?rev=770990&r1=770989&r2=770990&view=diff
= = = = = = = = ====================================================================== --- ofbiz/trunk/framework/minilang/src/org/ofbiz/minilang/method/ envops/Iterate.java (original) +++ ofbiz/trunk/framework/minilang/src/org/ofbiz/minilang/method/ envops/Iterate.java Sat May 2 22:37:08 2009
@@ -34,8 +34,6 @@
import org.ofbiz.minilang.method.MethodContext;
import org.ofbiz.minilang.method.MethodOperation;
import org.w3c.dom.Element;
-import org.w3c.dom.Node;
-import org.w3c.dom.NodeList;

/**
 * Process sub-operations for each entry in the list
@@ -103,7 +101,7 @@
                }
                return false;
            }
-        } else if (objList instanceof Iterable) {
+        } else {
Collection<Object> theList = UtilGenerics.checkList(objList);

            if (theList == null) {
@@ -123,17 +121,6 @@
                    return false;
                }
            }
-        } else if (objList instanceof NodeList) {
-            NodeList theList = (NodeList) objList;
-            for (int i = 0; i < theList.getLength(); i++) {
-                Node theEntry = theList.item(i);
-                entryAcsr.put(methodContext, theEntry);
-
-                if (!SimpleMethod.runSubOps(subOps, methodContext)) {
- // only return here if it returns false, otherwise just carry on
-                    return false;
-                }
-            }
        }
        entryAcsr.put(methodContext, oldEntryValue);
        return true;



Attachment: smime.p7s
Description: S/MIME cryptographic signature

Reply via email to