>>>>> On Mon, 27 Aug 2007 15:59:13 -0700, Ryan Lubke <Ryan.Lubke_at_Sun.COM> said:
RL> SECTION: Modified Files
RL> ----------------------------
RL> M src/javax/faces/component/UIData.java
RL> - add client ID caching scheme for non nested tables.
RL> In the non-nested case:
RL> the base client ID will not change, so store this in
RL> the StringBuilder. When a new per-row ID is needed
RL> append the row index and call toString() on the builder,
RL> and then reset the size of the builder to the base client
RL> ID + the naming container separator
Since diff doesn't always tell me what method I'm in, please say so
here. For example, say "in getClientId(), add client ID caching..."
Yeah, it should be obvious, but explicit is better.
RL> In the nested case, use the StringBuilder, but reset the
RL> length to 0 after each computed ID
RL> - In all cases where we call getChildren() or getFacets() call
RL> getChildCount() and getFacetCount() and only call the methods
RL> returning a collection when there are children or facets (reduces
RL> the chance of creating empty collections)
RL> - cache the result if isNestedWithinUIData since this is called multiple times
I'm really sorry, but with a change like this, I have to ask why there
is no addition to the automated tests? Unless there is some good reason
for there not to be one, I cannot give an r=edburns.
Some additional comments below.
RL> SECTION: Diffs
RL> ----------------------------
RL> Index: src/javax/faces/component/UIData.java
RL> ===================================================================
RL> RCS file: /cvs/javaserverfaces-sources/jsf-api/src/javax/faces/component/UIData.java,v
RL> retrieving revision 1.75.4.1
RL> diff -u -r1.75.4.1 UIData.java
RL> --- src/javax/faces/component/UIData.java 27 Aug 2007 04:57:28 -0000 1.75.4.1
RL> +++ src/javax/faces/component/UIData.java 27 Aug 2007 22:52:13 -0000
RL> @@ -177,6 +177,34 @@
RL> */
RL> private String var = null;
RL> +
RL> + /**
RL> + * <p>Holds the base client ID that will be used to generate per-row
RL> + * client IDs (this will be null if this UIData is nested within another).</p>
RL> + */
RL> + private String baseClientId;
If this is not a part of the saved state, mark it transient using the
"transient" java keyword. Also, I would like to have you explicitly
initialize this to "null" since you are counting on that being the
initial value in your new getClientId() method.
RL> +
RL> +
RL> + /**
RL> + * <p> Length of the cached <code>baseClientId</code> plus one for the
RL> + * NamingContainer.SEPARATOR_CHAR. </p>
RL> + */
RL> + private int baseClientIdLength;
Same here.
RL> +
RL> +
RL> + /**
RL> + * <p>StringBuilder used to build per-row client IDs.</p>
RL> + */
RL> + private StringBuilder clientIdBuilder;
And here.
RL> +
RL> +
RL> + /**
RL> + * <p>Flag indicating whether or not this UIData instance is nested
RL> + * within another UIData instance</p>
RL> + */
RL> + private Boolean isNested;
RL> +
RL> +
RL> // -------------------------------------------------------------- Properties
RL> @@ -680,11 +708,36 @@
RL> if (context == null) {
RL> throw new NullPointerException();
RL> }
RL> - String baseClientId = super.getClientId(context);
RL> +
RL> + if (baseClientId == null && clientIdBuilder == null) {
RL> + if (!isNestedWithinUIData()) {
RL> + clientIdBuilder = new StringBuilder(super.getClientId(context));
RL> + baseClientId = clientIdBuilder.toString();
RL> + baseClientIdLength = (baseClientId.length() + 1);
RL> + clientIdBuilder.append(NamingContainer.SEPARATOR_CHAR);
RL> + clientIdBuilder.setLength(baseClientIdLength);
RL> + } else {
RL> + clientIdBuilder = new StringBuilder();
RL> + }
RL> + }
I need some inline comments here. When doing optimizations, comments
are essential to maintainability.
RL> if (rowIndex >= 0) {
RL> - return (baseClientId + NamingContainer.SEPARATOR_CHAR + rowIndex);
RL> + String cid;
RL> + if (!isNestedWithinUIData()) {
RL> + cid = clientIdBuilder.append(rowIndex).toString();
RL> + clientIdBuilder.setLength(baseClientIdLength);
RL> + } else {
RL> + cid = clientIdBuilder.append(super.getClientId(context))
RL> + .append(NamingContainer.SEPARATOR_CHAR).append(rowIndex)
RL> + .toString();
RL> + clientIdBuilder.setLength(0);
RL> + }
RL> + return (cid);
RL> } else {
RL> - return (baseClientId);
RL> + if (!isNestedWithinUIData()) {
RL> + return (baseClientId);
RL> + } else {
RL> + return super.getClientId(context);
RL> + }
RL> }
RL> }
RL> @@ -777,7 +830,8 @@
RL> this.setRowIndex(newRow);
RL> if (this.isRowAvailable()) {
RL> found = super.invokeOnComponent(context,
RL> - clientId, callback);
RL> + clientId,
RL> + callback);
RL> }
RL> }
RL> }
RL> @@ -850,7 +904,6 @@
RL> FacesEvent rowEvent = revent.getFacesEvent();
RL> rowEvent.getComponent().broadcast(rowEvent);
RL> setRowIndex(oldRowIndex);
RL> - return;
RL> }
RL> @@ -917,8 +970,7 @@
RL> setDataModel(null); // Re-evaluate even with server-side state saving
RL> if (null == saved || !keepSaved(context)) {
RL> //noinspection CollectionWithoutInitialCapacity
RL> - saved =
RL> - new HashMap<String, SavedState>(); // We don't need saved state here
RL> + saved = new HashMap<String, SavedState>(); // We don't need saved state here
RL> }
RL> iterate(context, PhaseId.APPLY_REQUEST_VALUES);
RL> @@ -1103,47 +1155,43 @@
RL> // Process each facet of this component exactly once
RL> setRowIndex(-1);
RL> - Iterator facets = getFacets().keySet().iterator();
RL> - while (facets.hasNext()) {
RL> - UIComponent facet = getFacets().get(facets.next());
RL> - if (phaseId == PhaseId.APPLY_REQUEST_VALUES) {
RL> - facet.processDecodes(context);
RL> - } else if (phaseId == PhaseId.PROCESS_VALIDATIONS) {
RL> - facet.processValidators(context);
RL> - } else if (phaseId == PhaseId.UPDATE_MODEL_VALUES) {
RL> - facet.processUpdates(context);
RL> - } else {
RL> - throw new IllegalArgumentException();
RL> - }
RL> - }
RL> -
RL> - // Process each facet of our child UIColumn components exactly once
RL> - setRowIndex(-1);
RL> - Iterator columns = getChildren().iterator();
RL> - while (columns.hasNext()) {
RL> - UIComponent column = (UIComponent) columns.next();
RL> - if (!(column instanceof UIColumn)) {
RL> - continue;
RL> - }
RL> - if (!column.isRendered()) {
RL> - continue;
RL> - }
RL> - Iterator columnFacets = column.getFacets().keySet().iterator();
RL> - while (columnFacets.hasNext()) {
RL> - UIComponent columnFacet =
RL> - column.getFacets().get(columnFacets.next());
RL> + if (getFacetCount() > 0) {
RL> + for (UIComponent facet : getFacets().values()) {
RL> if (phaseId == PhaseId.APPLY_REQUEST_VALUES) {
RL> - columnFacet.processDecodes(context);
RL> + facet.processDecodes(context);
RL> } else if (phaseId == PhaseId.PROCESS_VALIDATIONS) {
RL> - columnFacet.processValidators(context);
RL> + facet.processValidators(context);
RL> } else if (phaseId == PhaseId.UPDATE_MODEL_VALUES) {
RL> - columnFacet.processUpdates(context);
RL> + facet.processUpdates(context);
RL> } else {
RL> throw new IllegalArgumentException();
RL> }
RL> }
RL> }
RL> + // Process each facet of our child UIColumn components exactly once
RL> + setRowIndex(-1);
RL> + if (getChildCount() > 0) {
RL> + for (UIComponent column : getChildren()) {
RL> + if (!(column instanceof UIColumn) || !column.isRendered()) {
RL> + continue;
RL> + }
RL> + if (column.getFacetCount() > 0) {
RL> + for (UIComponent columnFacet : column.getFacets().values()) {
RL> + if (phaseId == PhaseId.APPLY_REQUEST_VALUES) {
RL> + columnFacet.processDecodes(context);
RL> + } else if (phaseId == PhaseId.PROCESS_VALIDATIONS) {
RL> + columnFacet.processValidators(context);
RL> + } else if (phaseId == PhaseId.UPDATE_MODEL_VALUES) {
RL> + columnFacet.processUpdates(context);
RL> + } else {
RL> + throw new IllegalArgumentException();
RL> + }
RL> + }
RL> + }
RL> + }
RL> + }
RL> +
RL> // Iterate over our UIColumn children, once per row
RL> int processed = 0;
RL> int rowIndex = getFirst() - 1;
RL> @@ -1165,26 +1213,26 @@
RL> // Perform phase-specific processing as required
RL> // on the *children* of the UIColumn (facets have
RL> // been done a single time with rowIndex=-1 already)
RL> - Iterator kids = getChildren().iterator();
RL> - while (kids.hasNext()) {
RL> - UIComponent kid = (UIComponent) kids.next();
RL> - if (!(kid instanceof UIColumn)) {
RL> - continue;
RL> - }
RL> - Iterator grandkids = kid.getChildren().iterator();
RL> - while (grandkids.hasNext()) {
RL> - UIComponent grandkid = (UIComponent) grandkids.next();
RL> - if (!grandkid.isRendered()) {
RL> + if (getChildCount() > 0) {
RL> + for (UIComponent kid : getChildren()) {
RL> + if (!(kid instanceof UIColumn) || !kid.isRendered()) {
RL> continue;
RL> }
RL> - if (phaseId == PhaseId.APPLY_REQUEST_VALUES) {
RL> - grandkid.processDecodes(context);
RL> - } else if (phaseId == PhaseId.PROCESS_VALIDATIONS) {
RL> - grandkid.processValidators(context);
RL> - } else if (phaseId == PhaseId.UPDATE_MODEL_VALUES) {
RL> - grandkid.processUpdates(context);
RL> - } else {
RL> - throw new IllegalArgumentException();
RL> + if (kid.getChildCount() > 0) {
RL> + for (UIComponent grandkid : kid.getChildren()) {
RL> + if (!grandkid.isRendered()) {
RL> + continue;
RL> + }
RL> + if (phaseId == PhaseId.APPLY_REQUEST_VALUES) {
RL> + grandkid.processDecodes(context);
RL> + } else if (phaseId == PhaseId.PROCESS_VALIDATIONS) {
RL> + grandkid.processValidators(context);
RL> + } else if (phaseId == PhaseId.UPDATE_MODEL_VALUES) {
RL> + grandkid.processUpdates(context);
RL> + } else {
RL> + throw new IllegalArgumentException();
RL> + }
RL> + }
RL> }
RL> }
RL> }
RL> @@ -1216,9 +1264,7 @@
RL> */
RL> private boolean keepSaved(FacesContext context) {
RL> - Iterator clientIds = saved.keySet().iterator();
RL> - while (clientIds.hasNext()) {
RL> - String clientId = (String) clientIds.next();
RL> + for (String clientId : saved.keySet()) {
RL> Iterator messages = context.getMessages(clientId);
RL> while (messages.hasNext()) {
RL> FacesMessage message = (FacesMessage) messages.next();
RL> @@ -1232,14 +1278,22 @@
RL> }
RL> - private boolean isNestedWithinUIData() {
RL> - UIComponent parent = this;
RL> - while (null != (parent = parent.getParent())) {
RL> - if (parent instanceof UIData) {
RL> - return true;
RL> + private Boolean isNestedWithinUIData() {
RL> + if (isNested == null) {
RL> + UIComponent parent = this;
RL> + while (null != (parent = parent.getParent())) {
RL> + if (parent instanceof UIData) {
RL> + isNested = Boolean.TRUE;
RL> + break;
RL> + }
RL> + }
RL> + if (isNested == null) {
RL> + isNested = Boolean.FALSE;
RL> }
RL> + return isNested;
RL> + } else {
RL> + return isNested;
RL> }
RL> - return (false);
RL> }
RL> @@ -1250,11 +1304,11 @@
RL> private void restoreDescendantState() {
RL> FacesContext context = getFacesContext();
RL> - Iterator kids = getChildren().iterator();
RL> - while (kids.hasNext()) {
RL> - UIComponent kid = (UIComponent) kids.next();
RL> - if (kid instanceof UIColumn) {
RL> - restoreDescendantState(kid, context);
RL> + if (getChildCount() > 0) {
RL> + for (UIComponent kid : getChildren()) {
RL> + if (kid instanceof UIColumn) {
RL> + restoreDescendantState(kid, context);
RL> + }
RL> }
RL> }
RL> @@ -1292,16 +1346,16 @@
RL> }
RL> // Restore state for children of this component
RL> - Iterator kids = component.getChildren().iterator();
RL> - while (kids.hasNext()) {
RL> - restoreDescendantState((UIComponent) kids.next(), context);
RL> + if (component.getChildCount() > 0) {
RL> + for (UIComponent kid : component.getChildren()) {
RL> + restoreDescendantState(kid, context);
RL> + }
RL> }
RL> +
RL> // Restore state for facets of this component
RL> - Iterator facetNames = component.getFacets().keySet().iterator();
RL> - while (facetNames.hasNext()) {
RL> - UIComponent c = component.getFacet((String) facetNames.next());
RL> - if (c != null) {
RL> - restoreDescendantState(c, context);
RL> + if (component.getFacetCount() > 0) {
RL> + for (UIComponent facet : component.getFacets().values()) {
RL> + restoreDescendantState(facet, context);
RL> }
RL> }
RL> @@ -1315,11 +1369,11 @@
RL> private void saveDescendantState() {
RL> FacesContext context = getFacesContext();
RL> - Iterator kids = getChildren().iterator();
RL> - while (kids.hasNext()) {
RL> - UIComponent kid = (UIComponent) kids.next();
RL> - if (kid instanceof UIColumn) {
RL> - saveDescendantState(kid, context);
RL> + if (getChildCount() > 0) {
RL> + for (UIComponent kid : getChildren()) {
RL> + if (kid instanceof UIColumn) {
RL> + saveDescendantState(kid, context);
RL> + }
RL> }
RL> }
RL> @@ -1352,16 +1406,16 @@
RL> }
RL> // Save state for children of this component
RL> - Iterator kids = component.getChildren().iterator();
RL> - while (kids.hasNext()) {
RL> - saveDescendantState((UIComponent) kids.next(), context);
RL> + if (component.getChildCount() > 0) {
RL> + for (UIComponent uiComponent : component.getChildren()) {
RL> + saveDescendantState(uiComponent, context);
RL> + }
RL> }
RL> +
RL> // Save state for facets of this component
RL> - Iterator facetNames = component.getFacets().keySet().iterator();
RL> - while (facetNames.hasNext()) {
RL> - UIComponent c = component.getFacet((String) facetNames.next());
RL> - if (c != null) {
RL> - saveDescendantState(c, context);
RL> + if (component.getFacetCount() > 0) {
RL> + for (UIComponent facet : component.getFacets().values()) {
RL> + saveDescendantState(facet, context);
RL> }
RL> }
RL> ---------------------------------------------------------------------
RL> To unsubscribe, e-mail: dev-unsubscribe_at_javaserverfaces.dev.java.net
RL> For additional commands, e-mail: dev-help_at_javaserverfaces.dev.java.net
--
| ed.burns_at_sun.com | office: 408 884 9519 OR x31640
| homepage: | http://purl.oclc.org/NET/edburns/
| aim: edburns0sunw | iim: ed.burns_at_sun.com