[PATCH] OptionsTag.doEndTag -> efficiency using getIterator()

["Duff, Jason" <jd...@starkinvestments.com>]

When the logic falls into the section of code that uses "separate iterators"
to render the options there is the potential that the getIterator() is
called twice to get an iterator to the same list.  It is inefficient in the
sense that the underlying method that returns the collection for the
iterator may have significant processing involved.  From a purists
standpoint, in this situation, there is simply no reason to have to do this
twice, regardless of the degree of inefficiency.  This could be made much
more efficient by slightly modifying the code to use a combination of a flag
and only one iterator as noted in the diff included in the bottom of this
file...
Any questions/comments - or if you would like the .java file, let me know.
Thanks.
jason



--- //j/struts/src/share/org/apache/struts/taglib/html/OptionsTag.java  Sat
Jun  9 20:51:42 2001
+++ //j/struts/src/test/org/apache/struts/taglib/html/OptionsTag.java Wed
Aug  8 10:19:43 2001
@@ -252,22 +252,31 @@
         // Otherwise, use the separate iterators mode to render options
         else {
 
+              
               // Construct iterators for the values and labels collections
               Iterator valuesIterator = getIterator(name, property);
               Iterator labelsIterator = null;
-              if ((labelName == null) && (labelProperty == null))
-                  labelsIterator = getIterator(name, property); // Same
coll.
-              else
+              boolean labels = false; 
+              if ((labelName != null) || (labelProperty != null)) {
+                  labels = true;
                   labelsIterator = getIterator(labelName, labelProperty);
+              }
 
               // Render the options tags for each element of the values
coll.
               while (valuesIterator.hasNext()) {
                   String value = valuesIterator.next().toString();
                   String label = value;
-                  if (labelsIterator.hasNext())
+                  // Get the label values for each option
+                  if (labels == true) {
+                    if (labelsIterator.hasNext()) {
                       label = labelsIterator.next().toString();
-                  addOption(sb, value, label,
-                            selectTag.isMatched(value));
+                    }
+                  } else {
+                    // if the label property was not specified the label
will
+                    // be the same as the actual value for the option
+                    label = value;
+                  }
+                  addOption(sb, value, label, selectTag.isMatched(value));
               }
  }