dev@javaserverfaces.java.net

Re: [REVIEW] Minor improvements to UIData

From: Ryan Lubke <Ryan.Lubke_at_Sun.COM>
Date: Tue, 28 Aug 2007 09:42:08 -0700

More comments
> 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.
Why? UIData isn't serializable.

> 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.
>
Why, Object members will be automatically initialized to null when
instantiated.
> 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.
>
>
See above.
> RL> +
> RL> +
> RL> + /**
> RL> + * <p>StringBuilder used to build per-row client IDs.</p>
> RL> + */
> RL> + private StringBuilder clientIdBuilder;
>
> And here.
>
See above.
> 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.
>
OK.
> 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
>
>