Re: svn commit: r690740 - in /ofbiz/trunk/framework/widget/src/org/ofbiz/widget/menu: ModelMenu.java ModelMenuAction.java

[David E Jones]

Any reason to not use FastList?

-David


[hidden email] wrote:

> Author: adrianc
> Date: Sun Aug 31 10:29:38 2008
> New Revision: 690740
>
> URL: http://svn.apache.org/viewvc?rev=690740&view=rev
> Log:
> Menu widget extend actions bug fix, and some other work.
>
> I changed the LinkedList objects to ArrayList - because ArrayList is more appropriate for the "build once, read many" type use. It also takes less memory.
>
> Modified:
>     ofbiz/trunk/framework/widget/src/org/ofbiz/widget/menu/ModelMenu.java
>     ofbiz/trunk/framework/widget/src/org/ofbiz/widget/menu/ModelMenuAction.java
>
> Modified: ofbiz/trunk/framework/widget/src/org/ofbiz/widget/menu/ModelMenu.java
> URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/widget/src/org/ofbiz/widget/menu/ModelMenu.java?rev=690740&r1=690739&r2=690740&view=diff
> ==============================================================================
> --- ofbiz/trunk/framework/widget/src/org/ofbiz/widget/menu/ModelMenu.java (original)
> +++ ofbiz/trunk/framework/widget/src/org/ofbiz/widget/menu/ModelMenu.java Sun Aug 31 10:29:38 2008
> @@ -19,9 +19,8 @@
>  package org.ofbiz.widget.menu;
>  
>  import java.io.IOException;
> +import java.util.ArrayList;
>  import java.util.HashMap;
> -import java.util.Iterator;
> -import java.util.LinkedList;
>  import java.util.List;
>  import java.util.Map;
>  
> @@ -87,7 +86,7 @@
>       * necessary to use the Map. The Map is used when loading the menu definition to keep the
>       * list clean and implement the override features for item definitions.
>       */
> -    protected List<ModelMenuItem> menuItemList = new LinkedList<ModelMenuItem>();
> +    protected List<ModelMenuItem> menuItemList = new ArrayList<ModelMenuItem>();
>  
>      /** This Map is keyed with the item name and has a ModelMenuItem for the value; items
>       * with conditions will not be put in this Map so item definition overrides for items
> @@ -123,12 +122,10 @@
>              } else {
>                  // try to find a menu definition in the same file
>                  Element rootElement = menuElement.getOwnerDocument().getDocumentElement();
> -                List menuElements = UtilXml.childElementList(rootElement, "menu");
> +                List<? extends Element> menuElements = UtilXml.childElementList(rootElement, "menu");
>                  //Uncomment below to add support for abstract menus
>                  //menuElements.addAll(UtilXml.childElementList(rootElement, "abstract-menu"));
> -                Iterator menuElementIter = menuElements.iterator();
> -                while (menuElementIter.hasNext()) {
> -                    Element menuElementEntry = (Element) menuElementIter.next();
> +                for (Element menuElementEntry : menuElements) {
>                      if (menuElementEntry.getAttribute("name").equals(parentMenu)) {
>                          parent = new ModelMenu(menuElementEntry, delegator, dispatcher);
>                          break;
> @@ -170,10 +167,9 @@
>                  this.selectedMenuItemContextFieldName = parent.selectedMenuItemContextFieldName;
>                  this.menuContainerStyleExdr = parent.menuContainerStyleExdr;
>                  if (parent.actions != null) {
> -                    this.actions = new LinkedList<ModelMenuAction>();
> +                    this.actions = new ArrayList<ModelMenuAction>();
>                      this.actions.addAll(parent.actions);
>                  }
> -                this.actions = parent.actions;
>              }
>          }
>  
> @@ -243,14 +239,14 @@
>                  this.actions = ModelMenuAction.readSubActions(this, actionsElement);
>              } else {
>                  this.actions.addAll(ModelMenuAction.readSubActions(this, actionsElement));
> +                ArrayList<ModelMenuAction> actionsList = (ArrayList<ModelMenuAction>)this.actions;
> +                actionsList.trimToSize();
>              }
>          }
>  
>          // read in add item defs, add/override one by one using the menuItemList and menuItemMap
> -        List itemElements = UtilXml.childElementList(menuElement, "menu-item");
> -        Iterator itemElementIter = itemElements.iterator();
> -        while (itemElementIter.hasNext()) {
> -            Element itemElement = (Element) itemElementIter.next();
> +        List<? extends Element> itemElements = UtilXml.childElementList(menuElement, "menu-item");
> +        for (Element itemElement : itemElements) {
>              ModelMenuItem modelMenuItem = new ModelMenuItem(itemElement, this);
>              modelMenuItem = this.addUpdateMenuItem(modelMenuItem);
>          }
> @@ -286,9 +282,7 @@
>          ModelMenuItem existingMenuItem = null;
>          if (UtilValidate.isEmpty(contentId))
>              return existingMenuItem;
> -        Iterator iter = menuItemList.iterator();
> -        while (iter.hasNext()) {
> -            ModelMenuItem mi = (ModelMenuItem) iter.next();
> +        for (ModelMenuItem mi : this.menuItemList) {
>              String assocContentId = mi.getAssociatedContentId(context);
>              if (contentId.equals(assocContentId)) {
>                  existingMenuItem = mi;
> @@ -343,9 +337,7 @@
>              //Debug.logInfo("in ModelMenu, menuItemList:" + menuItemList, module);
>          // render each menuItem row, except hidden & ignored rows
>          //menuStringRenderer.renderFormatSimpleWrapperRows(writer, context, this);
> -        Iterator iter = menuItemList.iterator();
> -        while (iter.hasNext()) {
> -            ModelMenuItem item = (ModelMenuItem)iter.next();
> +        for (ModelMenuItem item : this.menuItemList) {
>              item.renderMenuItemString(writer, context, menuStringRenderer);
>          }
>  
> @@ -723,7 +715,7 @@
>          return this.defaultHideIfSelected;
>      }
>  
> -    public List getMenuItemList() {
> +    public List<ModelMenuItem> getMenuItemList() {
>          return menuItemList;
>      }
>  
>
> Modified: ofbiz/trunk/framework/widget/src/org/ofbiz/widget/menu/ModelMenuAction.java
> URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/widget/src/org/ofbiz/widget/menu/ModelMenuAction.java?rev=690740&r1=690739&r2=690740&view=diff
> ==============================================================================
> --- ofbiz/trunk/framework/widget/src/org/ofbiz/widget/menu/ModelMenuAction.java (original)
> +++ ofbiz/trunk/framework/widget/src/org/ofbiz/widget/menu/ModelMenuAction.java Sun Aug 31 10:29:38 2008
> @@ -19,8 +19,7 @@
>  package org.ofbiz.widget.menu;
>  
>  import java.text.MessageFormat;
> -import java.util.Iterator;
> -import java.util.LinkedList;
> +import java.util.ArrayList;
>  import java.util.List;
>  import java.util.Locale;
>  import java.util.Map;
> @@ -73,17 +72,14 @@
>      
>      public abstract void runAction(Map<String, Object> context);
>      
> -    public static List readSubActions(ModelMenuItem modelMenuItem, Element parentElement) {
> +    public static List<ModelMenuAction> readSubActions(ModelMenuItem modelMenuItem, Element parentElement) {
>          return readSubActions(modelMenuItem.getModelMenu(), parentElement);
>      }
>      
>      public static List<ModelMenuAction> readSubActions(ModelMenu modelMenu, Element parentElement) {
> -        List<ModelMenuAction> actions = new LinkedList<ModelMenuAction>();
> -        
> -        List actionElementList = UtilXml.childElementList(parentElement);
> -        Iterator actionElementIter = actionElementList.iterator();
> -        while (actionElementIter.hasNext()) {
> -            Element actionElement = (Element) actionElementIter.next();
> +        List<? extends Element> actionElementList = UtilXml.childElementList(parentElement);
> +        ArrayList<ModelMenuAction> actions = new ArrayList<ModelMenuAction>(actionElementList.size());
> +        for (Element actionElement : actionElementList) {
>              if ("set".equals(actionElement.getNodeName())) {
>                  actions.add(new SetField(modelMenu, actionElement));
>              } else if ("property-map".equals(actionElement.getNodeName())) {
> @@ -104,16 +100,13 @@
>                  throw new IllegalArgumentException("Action element not supported with name: " + actionElement.getNodeName());
>              }
>          }
> -        
> +        actions.trimToSize();
>          return actions;
>      }
>      
> -    public static void runSubActions(List actions, Map<String, Object> context) {
> +    public static void runSubActions(List<ModelMenuAction> actions, Map<String, Object> context) {
>          if (actions == null) return;
> -        
> -        Iterator actionIter = actions.iterator();
> -        while (actionIter.hasNext()) {
> -            ModelMenuAction action = (ModelMenuAction) actionIter.next();
> +        for (ModelMenuAction action : actions) {
>              if (Debug.verboseOn()) Debug.logVerbose("Running screen action " + action.getClass().getName(), module);
>              action.runAction(context);
>          }
>
>

[Adrian Crum-2]

I thought about that. I wasn't sure if there was a benefit. My understanding of Javalution is the speed improvement comes from building the objects, not from using them once they're built. I may be wrong - I haven't looked that closely at it.

If you believe a Fastlist would be better, then I would be happy to change it.

-Adrian


--- On Mon, 9/1/08, David E. Jones <[hidden email]> wrote:

> From: David E. Jones <[hidden email]>
> Subject: Re: svn commit: r690740 - in /ofbiz/trunk/framework/widget/src/org/ofbiz/widget/menu: ModelMenu.java ModelMenuAction.java
> To: [hidden email]
> Date: Monday, September 1, 2008, 6:32 AM
> Any reason to not use FastList?
>
> -David
>
>
> [hidden email] wrote:
> > Author: adrianc
> > Date: Sun Aug 31 10:29:38 2008
> > New Revision: 690740
> >
> > URL:
> http://svn.apache.org/viewvc?rev=690740&view=rev
> > Log:
> > Menu widget extend actions bug fix, and some other
> work.
> >
> > I changed the LinkedList objects to ArrayList -
> because ArrayList is more appropriate for the "build
> once, read many" type use. It also takes less memory.
> >
> > Modified:
> >    
> ofbiz/trunk/framework/widget/src/org/ofbiz/widget/menu/ModelMenu.java
> >    
> ofbiz/trunk/framework/widget/src/org/ofbiz/widget/menu/ModelMenuAction.java
> >
> > Modified:
> ofbiz/trunk/framework/widget/src/org/ofbiz/widget/menu/ModelMenu.java
> > URL:
> http://svn.apache.org/viewvc/ofbiz/trunk/framework/widget/src/org/ofbiz/widget/menu/ModelMenu.java?rev=690740&r1=690739&r2=690740&view=diff
> >
> ==============================================================================
> > ---
> ofbiz/trunk/framework/widget/src/org/ofbiz/widget/menu/ModelMenu.java
> (original)
> > +++
> ofbiz/trunk/framework/widget/src/org/ofbiz/widget/menu/ModelMenu.java
> Sun Aug 31 10:29:38 2008
> > @@ -19,9 +19,8 @@
> >  package org.ofbiz.widget.menu;
> >  
> >  import java.io.IOException;
> > +import java.util.ArrayList;
> >  import java.util.HashMap;
> > -import java.util.Iterator;
> > -import java.util.LinkedList;
> >  import java.util.List;
> >  import java.util.Map;
> >  
> > @@ -87,7 +86,7 @@
> >       * necessary to use the Map. The Map is used when
> loading the menu definition to keep the
> >       * list clean and implement the override features
> for item definitions.
> >       */
> > -    protected List<ModelMenuItem> menuItemList
> = new LinkedList<ModelMenuItem>();
> > +    protected List<ModelMenuItem> menuItemList
> = new ArrayList<ModelMenuItem>();
> >  
> >      /** This Map is keyed with the item name and has
> a ModelMenuItem for the value; items
> >       * with conditions will not be put in this Map so
> item definition overrides for items
> > @@ -123,12 +122,10 @@
> >              } else {
> >                  // try to find a menu definition in
> the same file
> >                  Element rootElement =
> menuElement.getOwnerDocument().getDocumentElement();
> > -                List menuElements =
> UtilXml.childElementList(rootElement, "menu");
> > +                List<? extends Element>
> menuElements = UtilXml.childElementList(rootElement,
> "menu");
> >                  //Uncomment below to add support for
> abstract menus
> >                
> //menuElements.addAll(UtilXml.childElementList(rootElement,
> "abstract-menu"));
> > -                Iterator menuElementIter =
> menuElements.iterator();
> > -                while (menuElementIter.hasNext()) {
> > -                    Element menuElementEntry =
> (Element) menuElementIter.next();
> > +                for (Element menuElementEntry :
> menuElements) {
> >                      if
> (menuElementEntry.getAttribute("name").equals(parentMenu))
> {
> >                          parent = new
> ModelMenu(menuElementEntry, delegator, dispatcher);
> >                          break;
> > @@ -170,10 +167,9 @@
> >                  this.selectedMenuItemContextFieldName
> = parent.selectedMenuItemContextFieldName;
> >                  this.menuContainerStyleExdr =
> parent.menuContainerStyleExdr;
> >                  if (parent.actions != null) {
> > -                    this.actions = new
> LinkedList<ModelMenuAction>();
> > +                    this.actions = new
> ArrayList<ModelMenuAction>();
> >                    
> this.actions.addAll(parent.actions);
> >                  }
> > -                this.actions = parent.actions;
> >              }
> >          }
> >  
> > @@ -243,14 +239,14 @@
> >                  this.actions =
> ModelMenuAction.readSubActions(this, actionsElement);
> >              } else {
> >                
> this.actions.addAll(ModelMenuAction.readSubActions(this,
> actionsElement));
> > +                ArrayList<ModelMenuAction>
> actionsList =
> (ArrayList<ModelMenuAction>)this.actions;
> > +                actionsList.trimToSize();
> >              }
> >          }
> >  
> >          // read in add item defs, add/override one by
> one using the menuItemList and menuItemMap
> > -        List itemElements =
> UtilXml.childElementList(menuElement,
> "menu-item");
> > -        Iterator itemElementIter =
> itemElements.iterator();
> > -        while (itemElementIter.hasNext()) {
> > -            Element itemElement = (Element)
> itemElementIter.next();
> > +        List<? extends Element> itemElements =
> UtilXml.childElementList(menuElement,
> "menu-item");
> > +        for (Element itemElement : itemElements) {
> >              ModelMenuItem modelMenuItem = new
> ModelMenuItem(itemElement, this);
> >              modelMenuItem =
> this.addUpdateMenuItem(modelMenuItem);
> >          }
> > @@ -286,9 +282,7 @@
> >          ModelMenuItem existingMenuItem = null;
> >          if (UtilValidate.isEmpty(contentId))
> >              return existingMenuItem;
> > -        Iterator iter = menuItemList.iterator();
> > -        while (iter.hasNext()) {
> > -            ModelMenuItem mi = (ModelMenuItem)
> iter.next();
> > +        for (ModelMenuItem mi : this.menuItemList) {
> >              String assocContentId =
> mi.getAssociatedContentId(context);
> >              if (contentId.equals(assocContentId)) {
> >                  existingMenuItem = mi;
> > @@ -343,9 +337,7 @@
> >              //Debug.logInfo("in ModelMenu,
> menuItemList:" + menuItemList, module);
> >          // render each menuItem row, except hidden
> & ignored rows
> >        
> //menuStringRenderer.renderFormatSimpleWrapperRows(writer,
> context, this);
> > -        Iterator iter = menuItemList.iterator();
> > -        while (iter.hasNext()) {
> > -            ModelMenuItem item =
> (ModelMenuItem)iter.next();
> > +        for (ModelMenuItem item : this.menuItemList)
> {
> >              item.renderMenuItemString(writer,
> context, menuStringRenderer);
> >          }
> >  
> > @@ -723,7 +715,7 @@
> >          return this.defaultHideIfSelected;
> >      }
> >  
> > -    public List getMenuItemList() {
> > +    public List<ModelMenuItem>
> getMenuItemList() {
> >          return menuItemList;
> >      }
> >  
> >
> > Modified:
> ofbiz/trunk/framework/widget/src/org/ofbiz/widget/menu/ModelMenuAction.java
> > URL:
> http://svn.apache.org/viewvc/ofbiz/trunk/framework/widget/src/org/ofbiz/widget/menu/ModelMenuAction.java?rev=690740&r1=690739&r2=690740&view=diff
> >
> ==============================================================================
> > ---
> ofbiz/trunk/framework/widget/src/org/ofbiz/widget/menu/ModelMenuAction.java
> (original)
> > +++
> ofbiz/trunk/framework/widget/src/org/ofbiz/widget/menu/ModelMenuAction.java
> Sun Aug 31 10:29:38 2008
> > @@ -19,8 +19,7 @@
> >  package org.ofbiz.widget.menu;
> >  
> >  import java.text.MessageFormat;
> > -import java.util.Iterator;
> > -import java.util.LinkedList;
> > +import java.util.ArrayList;
> >  import java.util.List;
> >  import java.util.Locale;
> >  import java.util.Map;
> > @@ -73,17 +72,14 @@
> >      
> >      public abstract void runAction(Map<String,
> Object> context);
> >      
> > -    public static List readSubActions(ModelMenuItem
> modelMenuItem, Element parentElement) {
> > +    public static List<ModelMenuAction>
> readSubActions(ModelMenuItem modelMenuItem, Element
> parentElement) {
> >          return
> readSubActions(modelMenuItem.getModelMenu(), parentElement);
> >      }
> >      
> >      public static List<ModelMenuAction>
> readSubActions(ModelMenu modelMenu, Element parentElement) {
> > -        List<ModelMenuAction> actions = new
> LinkedList<ModelMenuAction>();
> > -        
> > -        List actionElementList =
> UtilXml.childElementList(parentElement);
> > -        Iterator actionElementIter =
> actionElementList.iterator();
> > -        while (actionElementIter.hasNext()) {
> > -            Element actionElement = (Element)
> actionElementIter.next();
> > +        List<? extends Element>
> actionElementList = UtilXml.childElementList(parentElement);
> > +        ArrayList<ModelMenuAction> actions =
> new
> ArrayList<ModelMenuAction>(actionElementList.size());
> > +        for (Element actionElement :
> actionElementList) {
> >              if
> ("set".equals(actionElement.getNodeName())) {
> >                  actions.add(new SetField(modelMenu,
> actionElement));
> >              } else if
> ("property-map".equals(actionElement.getNodeName()))
> {
> > @@ -104,16 +100,13 @@
> >                  throw new
> IllegalArgumentException("Action element not supported
> with name: " + actionElement.getNodeName());
> >              }
> >          }
> > -        
> > +        actions.trimToSize();
> >          return actions;
> >      }
> >      
> > -    public static void runSubActions(List actions,
> Map<String, Object> context) {
> > +    public static void
> runSubActions(List<ModelMenuAction> actions,
> Map<String, Object> context) {
> >          if (actions == null) return;
> > -        
> > -        Iterator actionIter = actions.iterator();
> > -        while (actionIter.hasNext()) {
> > -            ModelMenuAction action =
> (ModelMenuAction) actionIter.next();
> > +        for (ModelMenuAction action : actions) {
> >              if (Debug.verboseOn())
> Debug.logVerbose("Running screen action " +
> action.getClass().getName(), module);
> >              action.runAction(context);
> >          }
> >
> >