You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ofbiz.apache.org by "David E. Jones" <jo...@hotwaxmedia.com> on 2008/09/01 15:32:49 UTC
Re: svn commit: r690740 - in /ofbiz/trunk/framework/widget/src/org/ofbiz/widget/menu:
ModelMenu.java ModelMenuAction.java
Any reason to not use FastList?
-David
adrianc@apache.org 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);
> }
>
>
Re: svn commit: r690740 - in /ofbiz/trunk/framework/widget/src/org/ofbiz/widget/menu: ModelMenu.java ModelMenuAction.java
Posted by Adrian Crum <ad...@yahoo.com>.
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 <jo...@hotwaxmedia.com> wrote:
> From: David E. Jones <jo...@hotwaxmedia.com>
> Subject: Re: svn commit: r690740 - in /ofbiz/trunk/framework/widget/src/org/ofbiz/widget/menu: ModelMenu.java ModelMenuAction.java
> To: dev@ofbiz.apache.org
> Date: Monday, September 1, 2008, 6:32 AM
> Any reason to not use FastList?
>
> -David
>
>
> adrianc@apache.org 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);
> > }
> >
> >