44950 - fixed HSSFFormulaEvaluator.evaluateInCell() and Area3DEval.getValue() also added validation for number of elements in AreaEvals

git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@654356 13f79535-47bb-0310-9956-ffa450edef68

5 files changed, 140 insertions, 154 deletions

diff --git a/src/java/org/apache/poi/hssf/record/formula/eval/Area2DEval.java b/src/java/org/apache/poi/hssf/record/formula/eval/Area2DEval.java
index 4b9a64c1c0..5ae98b39fe 100644
--- a/src/java/org/apache/poi/hssf/record/formula/eval/Area2DEval.java
+++ b/src/java/org/apache/poi/hssf/record/formula/eval/Area2DEval.java
@@ -22,79 +22,11 @@ import org.apache.poi.hssf.record.formula.Ptg;
 
 /**
  * @author Amol S. Deshmukh < amolweb at ya hoo dot com >
- *   
+ * 
  */
-public final class Area2DEval implements AreaEval {
-// TODO -refactor with Area3DEval
-    private final AreaPtg _delegate;
+public final class Area2DEval extends AreaEvalBase {
 
-    private final ValueEval[] _values;
-
-    public Area2DEval(Ptg ptg, ValueEval[] values) {
-        if(ptg == null) {
-            throw new IllegalArgumentException("ptg must not be null");
-        }
-        if(values == null) {
-            throw new IllegalArgumentException("values must not be null");
-        }
-        for(int i=values.length-1; i>=0; i--) {
-            if(values[i] == null) {
-                throw new IllegalArgumentException("value array elements must not be null");
-            }
-        }
-        // TODO - check size of array vs size of AreaPtg
-        _delegate = (AreaPtg) ptg;
-        _values = values;
-    }
-
-    public int getFirstColumn() {
-        return _delegate.getFirstColumn();
-    }
-
-    public int getFirstRow() {
-        return _delegate.getFirstRow();
-    }
-
-    public int getLastColumn() {
-        return _delegate.getLastColumn();
-    }
-
-    public int getLastRow() {
-        return _delegate.getLastRow();
-    }
-
-    public ValueEval[] getValues() {
-        return _values;
-    }
-    
-    public ValueEval getValueAt(int row, int col) {
-        ValueEval retval;
-        int index = ((row-getFirstRow())*(getLastColumn()-getFirstColumn()+1))+(col-getFirstColumn());
-        if (index <0 || index >= _values.length)
-            retval = ErrorEval.VALUE_INVALID;
-        else 
-            retval = _values[index];
-        return retval;
-    }
-    
-    public boolean contains(int row, int col) {
-        return (getFirstRow() <= row) && (getLastRow() >= row) 
-            && (getFirstColumn() <= col) && (getLastColumn() >= col);
-    }
-    
-    public boolean containsRow(int row) {
-        return (getFirstRow() <= row) && (getLastRow() >= row);
-    }
-    
-    public boolean containsColumn(short col) {
-        return (getFirstColumn() <= col) && (getLastColumn() >= col);
-    }
-    
-    public boolean isColumn() {
-        return _delegate.getFirstColumn() == _delegate.getLastColumn();
-    }
-
-    public boolean isRow() {
-        return _delegate.getFirstRow() == _delegate.getLastRow();
-    }
-}
+	public Area2DEval(Ptg ptg, ValueEval[] values) {
+		super((AreaPtg) ptg, values);
+	}
+}
\ No newline at end of file
diff --git a/src/java/org/apache/poi/hssf/record/formula/eval/Area3DEval.java b/src/java/org/apache/poi/hssf/record/formula/eval/Area3DEval.java
index 2f539142d1..89209e21b6 100644
--- a/src/java/org/apache/poi/hssf/record/formula/eval/Area3DEval.java
+++ b/src/java/org/apache/poi/hssf/record/formula/eval/Area3DEval.java
@@ -22,84 +22,18 @@ import org.apache.poi.hssf.record.formula.Ptg;
 
 /**
  * @author Amol S. Deshmukh &lt; amolweb at ya hoo dot com &gt;
- *  
+ * 
  */
-public final class Area3DEval implements AreaEval {
-	// TODO -refactor with Area3DEval
-    private final Area3DPtg _delegate;
+public final class Area3DEval extends AreaEvalBase {
 
-    private final ValueEval[] _values;
+	private final int _externSheetIndex;
 
-    public Area3DEval(Ptg ptg, ValueEval[] values) {
-        if(ptg == null) {
-            throw new IllegalArgumentException("ptg must not be null");
-        }
-        if(values == null) {
-            throw new IllegalArgumentException("values must not be null");
-        }
-        for(int i=values.length-1; i>=0; i--) {
-            if(values[i] == null) {
-                throw new IllegalArgumentException("value array elements must not be null");
-            }
-        }
-        // TODO - check size of array vs size of AreaPtg
-        _values = values;
-        _delegate = (Area3DPtg) ptg;
-    }
+	public Area3DEval(Ptg ptg, ValueEval[] values) {
+		super((Area3DPtg) ptg, values);
+		_externSheetIndex = ((Area3DPtg) ptg).getExternSheetIndex();
+	}
 
-    public int getFirstColumn() {
-        return _delegate.getFirstColumn();
-    }
-
-    public int getFirstRow() {
-        return _delegate.getFirstRow();
-    }
-
-    public int getLastColumn() {
-        return (short) _delegate.getLastColumn();
-    }
-
-    public int getLastRow() {
-        return _delegate.getLastRow();
-    }
-
-    public ValueEval[] getValues() {
-        return _values;
-    }
-    
-    public ValueEval getValueAt(int row, int col) {
-        ValueEval retval;
-        int index = (row-getFirstRow())*(col-getFirstColumn());
-        if (index <0 || index >= _values.length)
-            retval = ErrorEval.VALUE_INVALID;
-        else 
-            retval = _values[index];
-        return retval;
-    }
-    
-    public boolean contains(int row, int col) {
-        return (getFirstRow() <= row) && (getLastRow() >= row) 
-            && (getFirstColumn() <= col) && (getLastColumn() >= col);
-    }
-    
-    public boolean containsRow(int row) {
-        return (getFirstRow() <= row) && (getLastRow() >= row);
-    }
-    
-    public boolean containsColumn(short col) {
-        return (getFirstColumn() <= col) && (getLastColumn() >= col);
-    }
-    
-    
-    public boolean isColumn() {
-        return _delegate.getFirstColumn() == _delegate.getLastColumn();
-    }
-
-    public boolean isRow() {
-        return _delegate.getFirstRow() == _delegate.getLastRow();
-    }
-
-    public int getExternSheetIndex() {
-        return _delegate.getExternSheetIndex();
-    }
+	public int getExternSheetIndex() {
+		return _externSheetIndex;
+	}
 }
diff --git a/src/java/org/apache/poi/hssf/record/formula/eval/AreaEvalBase.java b/src/java/org/apache/poi/hssf/record/formula/eval/AreaEvalBase.java
new file mode 100644
index 0000000000..9436ae0aa9
--- /dev/null
+++ b/src/java/org/apache/poi/hssf/record/formula/eval/AreaEvalBase.java
@@ -0,0 +1,122 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.poi.hssf.record.formula.eval;
+
+import org.apache.poi.hssf.record.formula.AreaI;
+
+/**
+ * @author Josh Micich
+ */
+abstract class AreaEvalBase implements AreaEval {
+
+	private final int _firstColumn;
+	private final int _firstRow;
+	private final int _lastColumn;
+	private final int _lastRow;
+	private final ValueEval[] _values;
+	private final int _nColumns;
+	private final int _nRows;
+
+	protected AreaEvalBase(AreaI ptg, ValueEval[] values) {
+		if (values == null) {
+			throw new IllegalArgumentException("values must not be null");
+		}
+		_firstRow = ptg.getFirstRow();
+		_firstColumn = ptg.getFirstColumn();
+		_lastRow = ptg.getLastRow();
+		_lastColumn = ptg.getLastColumn();
+		
+		_nColumns = _lastColumn - _firstColumn + 1;
+		_nRows = _lastRow - _firstRow + 1;
+		
+		int expectedItemCount = _nRows * _nColumns;
+		if ((values.length != expectedItemCount)) {
+			// Note - this math may need alteration when POI starts to support full column or full row refs
+			throw new IllegalArgumentException("Array size should be (" + expectedItemCount
+					+ ") but was (" + values.length + ")");
+		}
+		
+
+
+		for (int i = values.length - 1; i >= 0; i--) {
+			if (values[i] == null) {
+				throw new IllegalArgumentException("value array elements must not be null");
+			}
+		}
+		_values = values;
+	}
+
+	public final int getFirstColumn() {
+		return _firstColumn;
+	}
+
+	public final int getFirstRow() {
+		return _firstRow;
+	}
+
+	public final int getLastColumn() {
+		return _lastColumn;
+	}
+
+	public final int getLastRow() {
+		return _lastRow;
+	}
+
+	public final ValueEval[] getValues() {
+		// TODO - clone() - but some junits rely on not cloning at the moment
+		return _values;
+	}
+
+	public final ValueEval getValueAt(int row, int col) {
+		int rowOffsetIx = row - _firstRow;
+		int colOffsetIx = col - _firstColumn;
+		
+		if(rowOffsetIx < 0 || rowOffsetIx >= _nRows) {
+			throw new IllegalArgumentException("Specified row index (" + row 
+					+ ") is outside the allowed range (" + _firstRow + ".." + _lastRow + ")");
+		}
+		if(colOffsetIx < 0 || colOffsetIx >= _nColumns) {
+			throw new IllegalArgumentException("Specified column index (" + col 
+					+ ") is outside the allowed range (" + _firstColumn + ".." + col + ")");
+		}
+
+		int index = rowOffsetIx * _nColumns + colOffsetIx;
+		return _values[index];
+	}
+
+	public final boolean contains(int row, int col) {
+		return _firstRow <= row && _lastRow >= row 
+			&& _firstColumn <= col && _lastColumn >= col;
+	}
+
+	public final boolean containsRow(int row) {
+		return (_firstRow <= row) && (_lastRow >= row);
+	}
+
+	public final boolean containsColumn(short col) {
+		return (_firstColumn <= col) && (_lastColumn >= col);
+	}
+
+	public final boolean isColumn() {
+		return _firstColumn == _lastColumn;
+	}
+
+	public final boolean isRow() {
+		return _firstRow == _lastRow;
+	}
+}
diff --git a/src/java/org/apache/poi/hssf/usermodel/HSSFCell.java b/src/java/org/apache/poi/hssf/usermodel/HSSFCell.java
index 81a3382567..7d2a53d207 100644
--- a/src/java/org/apache/poi/hssf/usermodel/HSSFCell.java
+++ b/src/java/org/apache/poi/hssf/usermodel/HSSFCell.java
@@ -817,8 +817,7 @@ public class HSSFCell
         int row=record.getRow();
         short col=record.getColumn();
         short styleIndex=record.getXFIndex();
-        if ((cellType != CELL_TYPE_ERROR) && (cellType != CELL_TYPE_FORMULA))
-        {
+        if (cellType != CELL_TYPE_ERROR) {
             setCellType(CELL_TYPE_ERROR, false, row, col, styleIndex);
         }
         (( BoolErrRecord ) record).setValue(value);
diff --git a/src/java/org/apache/poi/hssf/usermodel/HSSFFormulaEvaluator.java b/src/java/org/apache/poi/hssf/usermodel/HSSFFormulaEvaluator.java
index 0a9c3dfa80..cf4f83b311 100644
--- a/src/java/org/apache/poi/hssf/usermodel/HSSFFormulaEvaluator.java
+++ b/src/java/org/apache/poi/hssf/usermodel/HSSFFormulaEvaluator.java
@@ -232,8 +232,7 @@ public class HSSFFormulaEvaluator {
                     cell.setCellValue(cv.getBooleanValue());
                     break;
                 case HSSFCell.CELL_TYPE_ERROR:
-                    cell.setCellType(HSSFCell.CELL_TYPE_ERROR);
-                    cell.setCellValue(cv.getErrorValue());
+                    cell.setCellErrorValue(cv.getErrorValue());
                     break;
                 case HSSFCell.CELL_TYPE_NUMERIC:
                     cell.setCellType(HSSFCell.CELL_TYPE_NUMERIC);