javaserverfaces
  1. javaserverfaces
  2. JAVASERVERFACES-2244

UIInput and UIData create unnecessary state array

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 2.1.3, 2.1.4
    • Fix Version/s: None
    • Component/s: state saving
    • Labels:
      None

      Description

      Similar to issue JAVASERVERFACES-2203, UIInput and UIData also create unnecessary state arrays:

      The most obvious is UIInput:

      UIInput.java
      public Object saveState(FacesContext context) {
      
          if (context == null) {
              throw new NullPointerException();
          }
          Object[] values = null;
          if (values == null) {
              values = new Object[4];
          }
      
          values[0] = super.saveState(context);
          values[1] = emptyStringIsNull;
          values[2] = validateEmptyFields;
          values[3] = ((validators != null) ? validators.saveState(context) : null);
          return (values);
       }
      

      Should become something like:

      UIInput.java
       public Object saveState(FacesContext context) {
      
              if (context == null) {
                  throw new NullPointerException();
              }
              
              Object[] values = null;
              Object superState = super.saveState(context);
              Object attachedState = ((validators != null) ? validators.saveState(context) : null);
      
              if (superState != null || emptyStringIsNull != null || validateEmptyFields != null || attachedState != null) {
                  values = new Object[] {superState, emptyStringIsNull, validateEmptyFields, attachedState};
              }
      
              return (values);
       }
      

      UIData is a little less obvious. It has a guard, but still calculates the super state twice and the part where the state has not been marked does suffer directly from the problem.

      UIData.java
       @Override
       public Object saveState(FacesContext context)
       {
          if (initialStateMarked())
          {
              Object parentSaved = super.saveState(context);
              if (parentSaved == null &&_rowDeltaStates.isEmpty())
              {
                  return null;
              }
              else
              {
                  Object values[] = new Object[2];
                  values[0] = super.saveState(context);
                  values[1] = UIComponentBase.saveAttachedState(context, _rowDeltaStates);
                  return values; 
              }
          }
          else
          {
                  Object values[] = new Object[2];
                  values[0] = super.saveState(context);
                  values[1] = UIComponentBase.saveAttachedState(context, _rowDeltaStates);
                  return values;
          }
      }
      

      Could become:

      UIData.java
       public Object saveState(FacesContext context) {
             
             Object superState = super.saveState(context);
             if (initialStateMarked() && superState == null && _rowDeltaStates.isEmpty()) {        
                 return null;
             } 
             
             Object values[] = null;          
             Object attachedState = UIComponentBase.saveAttachedState(context, _rowDeltaStates);
             if (superState != null || attachedState != null) {
                 values = new Object[] {superState, attachedState};
             }
                     
             return values;    
       }
      

      UIViewRoot has the guard now, but still defines values as an instance variable. In changeset 8600 Ed Burns had moved the same variable in UIInput to a local variable. It's perhaps better to do that here as well.

      I've attached a patch against the mojarra trunk (2.2). All tests pass after the patch, although one test had to be modified (a test that checks the returned state array for attached state now has to test the entire array for null instead of the third element).

        Issue Links

          Activity

            People

            • Assignee:
              Manfred Riem
              Reporter:
              arjan tijms
            • Votes:
              10 Vote for this issue
              Watchers:
              5 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: