From 376fd3259b2f508a2b2c85728ad3e93116a9fb34 Mon Sep 17 00:00:00 2001 From: Josh Micich Date: Sun, 6 Apr 2008 20:27:40 +0000 Subject: [PATCH] Fix for bug 44708. XSSFCell.getCellType() now returns CELL_TYPE_BLANK for numeric cells with no value. git-svn-id: https://svn.apache.org/repos/asf/poi/branches/ooxml@645298 13f79535-47bb-0310-9956-ffa450edef68 --- build.xml | 3 +- .../org/apache/poi/ss/usermodel/Cell.java | 3 +- .../apache/poi/xssf/usermodel/XSSFCell.java | 215 ++++++++++-------- .../org/apache/poi/xssf/AllXSSFTests.java | 54 +++++ .../xssf/usermodel/AllXSSFUsermodelTests.java | 56 +++++ .../usermodel/TestFormulaEvaluatorOnXSSF.java | 37 ++- .../poi/xssf/usermodel/TestXSSFRow.java | 115 +++++----- 7 files changed, 327 insertions(+), 156 deletions(-) create mode 100644 src/ooxml/testcases/org/apache/poi/xssf/AllXSSFTests.java create mode 100644 src/ooxml/testcases/org/apache/poi/xssf/usermodel/AllXSSFUsermodelTests.java diff --git a/build.xml b/build.xml index 5b46396f3..2794f6acd 100644 --- a/build.xml +++ b/build.xml @@ -236,6 +236,7 @@ under the License. + @@ -790,7 +791,7 @@ under the License. - + diff --git a/src/ooxml/interfaces-jdk15/org/apache/poi/ss/usermodel/Cell.java b/src/ooxml/interfaces-jdk15/org/apache/poi/ss/usermodel/Cell.java index b63f60465..9a5def70c 100644 --- a/src/ooxml/interfaces-jdk15/org/apache/poi/ss/usermodel/Cell.java +++ b/src/ooxml/interfaces-jdk15/org/apache/poi/ss/usermodel/Cell.java @@ -117,11 +117,12 @@ public interface Cell { void setCellType(int cellType); /** - * get the cells type (numeric, formula or string) + * @return the cell's type (e.g. numeric, formula or string) * @see #CELL_TYPE_STRING * @see #CELL_TYPE_NUMERIC * @see #CELL_TYPE_FORMULA * @see #CELL_TYPE_BOOLEAN + * @see #CELL_TYPE_BLANK * @see #CELL_TYPE_ERROR */ diff --git a/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFCell.java b/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFCell.java index 3606415b8..3332dd6ff 100644 --- a/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFCell.java +++ b/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFCell.java @@ -35,8 +35,10 @@ import org.openxmlformats.schemas.spreadsheetml.x2006.main.CTCell; import org.openxmlformats.schemas.spreadsheetml.x2006.main.CTCellFormula; import org.openxmlformats.schemas.spreadsheetml.x2006.main.STCellType; - -public class XSSFCell implements Cell { +/** + * + */ +public final class XSSFCell implements Cell { private static final String FALSE_AS_STRING = "0"; private static final String TRUE_AS_STRING = "1"; @@ -45,9 +47,9 @@ public class XSSFCell implements Cell { private int cellNum; private SharedStringSource sharedStringSource; private StylesSource stylesSource; - + private POILogger logger = POILogFactory.getLogger(XSSFCell.class); - + /** * Create a new XSSFCell. This method is protected to be used only by * tests. @@ -55,7 +57,7 @@ public class XSSFCell implements Cell { protected XSSFCell(XSSFRow row) { this(row, CTCell.Factory.newInstance()); } - + public XSSFCell(XSSFRow row, CTCell cell) { this.cell = cell; this.row = row; @@ -65,31 +67,31 @@ public class XSSFCell implements Cell { this.sharedStringSource = row.getSheet().getWorkbook().getSharedStringSource(); this.stylesSource = row.getSheet().getWorkbook().getStylesSource(); } - + protected SharedStringSource getSharedStringSource() { return this.sharedStringSource; } protected StylesSource getStylesSource() { - return this.stylesSource; + return this.stylesSource; } public boolean getBooleanCellValue() { - if (STCellType.B != cell.getT()) { + if (STCellType.B != cell.getT()) { throw new NumberFormatException("You cannot get a boolean value from a non-boolean cell"); } if (cell.isSetV()) { - return (TRUE_AS_STRING.equals(this.cell.getV())); + return (TRUE_AS_STRING.equals(this.cell.getV())); } - + return false; } public Comment getCellComment() { - return row.getSheet().getCellComment(row.getRowNum(), getCellNum()); + return row.getSheet().getCellComment(row.getRowNum(), getCellNum()); } public String getCellFormula() { - if(this.cell.getF() == null) { + if(this.cell.getF() == null) { throw new NumberFormatException("You cannot get a formula from a non-formula cell"); } return this.cell.getF().getStringValue(); @@ -100,24 +102,32 @@ public class XSSFCell implements Cell { } public CellStyle getCellStyle() { - // Zero is the empty default - if(this.cell.getS() > 0) { - return stylesSource.getStyleAt(this.cell.getS()); - } - return null; + // Zero is the empty default + if(this.cell.getS() > 0) { + return stylesSource.getStyleAt(this.cell.getS()); + } + return null; } public int getCellType() { - // Detecting formulas is quite pesky, - // as they don't get their type set - if(this.cell.getF() != null) { - return CELL_TYPE_FORMULA; - } - + // Detecting formulas is quite pesky, + // as they don't get their type set + if(this.cell.getF() != null) { + return CELL_TYPE_FORMULA; + } + switch (this.cell.getT().intValue()) { case STCellType.INT_B: return CELL_TYPE_BOOLEAN; case STCellType.INT_N: + if(!cell.isSetV()) { + // ooxml does have a separate cell type of 'blank'. A blank cell gets encoded as + // (either not present or) a numeric cell with no value set. + // The formula evaluator (and perhaps other clients of this interface) needs to + // distinguish blank values which sometimes get translated into zero and sometimes + // empty string, depending on context + return CELL_TYPE_BLANK; + } return CELL_TYPE_NUMERIC; case STCellType.INT_E: return CELL_TYPE_ERROR; @@ -148,11 +158,11 @@ public class XSSFCell implements Cell { * Returns the error message, such as #VALUE! */ public String getErrorCellString() { - if (STCellType.E != cell.getT()) { + if (STCellType.E != cell.getT()) { throw new NumberFormatException("You cannot get a error value from a non-error cell"); } if (this.cell.isSetV()) { - return this.cell.getV(); + return this.cell.getV(); } return null; } @@ -161,42 +171,59 @@ public class XSSFCell implements Cell { * HSSFCell does. See {@link Cell} for details */ public byte getErrorCellValue() { - if (STCellType.E != cell.getT()) { + if (STCellType.E != cell.getT()) { throw new NumberFormatException("You cannot get a error value from a non-error cell"); } if (this.cell.isSetV()) { - String errS = this.cell.getV(); - if(errS.equals(Cell.ERROR_NULL.getStringRepr())) { - return Cell.ERROR_NULL.getType(); - } - if(errS.equals(Cell.ERROR_DIV0.getStringRepr())) { - return Cell.ERROR_DIV0.getType(); - } - if(errS.equals(Cell.ERROR_VALUE.getStringRepr())) { - return Cell.ERROR_VALUE.getType(); - } - if(errS.equals(Cell.ERROR_REF.getStringRepr())) { - return Cell.ERROR_REF.getType(); - } - if(errS.equals(Cell.ERROR_NAME.getStringRepr())) { - return Cell.ERROR_NAME.getType(); - } - if(errS.equals(Cell.ERROR_NUM.getStringRepr())) { - return Cell.ERROR_NUM.getType(); - } - return Cell.ERROR_NA.getType(); + String errS = this.cell.getV(); + if(errS.equals(Cell.ERROR_NULL.getStringRepr())) { + return Cell.ERROR_NULL.getType(); + } + if(errS.equals(Cell.ERROR_DIV0.getStringRepr())) { + return Cell.ERROR_DIV0.getType(); + } + if(errS.equals(Cell.ERROR_VALUE.getStringRepr())) { + return Cell.ERROR_VALUE.getType(); + } + if(errS.equals(Cell.ERROR_REF.getStringRepr())) { + return Cell.ERROR_REF.getType(); + } + if(errS.equals(Cell.ERROR_NAME.getStringRepr())) { + return Cell.ERROR_NAME.getType(); + } + if(errS.equals(Cell.ERROR_NUM.getStringRepr())) { + return Cell.ERROR_NUM.getType(); + } + return Cell.ERROR_NA.getType(); } return 0; } public double getNumericCellValue() { - if (STCellType.N != cell.getT() && STCellType.STR != cell.getT()) { + if (STCellType.N != cell.getT() && STCellType.STR != cell.getT()) { throw new NumberFormatException("You cannot get a numeric value from a non-numeric cell"); } if (this.cell.isSetV()) { return Double.parseDouble(this.cell.getV()); } - return Double.NaN; + // else - cell is blank. + + // TODO - behaviour in the case of blank cells. + // Revise spec, choose best alternative below, and comment why. + if (true) { + // returning NaN from a blank cell seems wrong + // there are a few junits which assert this behaviour, though. + return Double.NaN; + } + if (true) { + // zero is probably a more reasonable value. + return 0.0; + } else { + // or perhaps disallow reading value from blank cell. + throw new RuntimeException("You cannot get a numeric value from a blank cell"); + } + // Note - it would be nice if the behaviour is consistent with getRichStringCellValue + // (i.e. whether to return empty string or throw exception). } public RichTextString getRichStringCellValue() { @@ -219,34 +246,34 @@ public class XSSFCell implements Cell { } public void setAsActiveCell() { - row.getSheet().setActiveCell(cell.getR()); + row.getSheet().setActiveCell(cell.getR()); } public void setCellComment(Comment comment) { - String cellRef = - new CellReference(row.getRowNum(), getCellNum()).formatAsString(); - row.getSheet().setCellComment(cellRef, (XSSFComment)comment); + String cellRef = + new CellReference(row.getRowNum(), getCellNum()).formatAsString(); + row.getSheet().setCellComment(cellRef, (XSSFComment)comment); } public void setCellErrorValue(byte value) { - if(value == Cell.ERROR_DIV0.getType()) { - setCellErrorValue(Cell.ERROR_DIV0); - } else if(value == Cell.ERROR_NA.getType()) { - setCellErrorValue(Cell.ERROR_NA); - } else if(value == Cell.ERROR_NAME.getType()) { - setCellErrorValue(Cell.ERROR_NAME); - } else if(value == Cell.ERROR_NULL.getType()) { - setCellErrorValue(Cell.ERROR_NULL); - } else if(value == Cell.ERROR_NUM.getType()) { - setCellErrorValue(Cell.ERROR_NUM); - } else if(value == Cell.ERROR_REF.getType()) { - setCellErrorValue(Cell.ERROR_REF); - } else if(value == Cell.ERROR_VALUE.getType()) { - setCellErrorValue(Cell.ERROR_VALUE); - } else { - logger.log(POILogger.WARN, "Unknown error type " + value + " specified, treating as #N/A"); - setCellErrorValue(Cell.ERROR_NA); - } + if(value == Cell.ERROR_DIV0.getType()) { + setCellErrorValue(Cell.ERROR_DIV0); + } else if(value == Cell.ERROR_NA.getType()) { + setCellErrorValue(Cell.ERROR_NA); + } else if(value == Cell.ERROR_NAME.getType()) { + setCellErrorValue(Cell.ERROR_NAME); + } else if(value == Cell.ERROR_NULL.getType()) { + setCellErrorValue(Cell.ERROR_NULL); + } else if(value == Cell.ERROR_NUM.getType()) { + setCellErrorValue(Cell.ERROR_NUM); + } else if(value == Cell.ERROR_REF.getType()) { + setCellErrorValue(Cell.ERROR_REF); + } else if(value == Cell.ERROR_VALUE.getType()) { + setCellErrorValue(Cell.ERROR_VALUE); + } else { + logger.log(POILogger.WARN, "Unknown error type " + value + " specified, treating as #N/A"); + setCellErrorValue(Cell.ERROR_NA); + } } public void setCellErrorValue(CELL_ERROR_TYPE errorType) { if ((this.cell.getT() != STCellType.E) && (this.cell.getT() != STCellType.STR)) @@ -255,8 +282,8 @@ public class XSSFCell implements Cell { } this.cell.setV( errorType.getStringRepr() ); } - - + + public void setCellFormula(String formula) { if (this.cell.getT() != STCellType.STR) { @@ -269,11 +296,11 @@ public class XSSFCell implements Cell { if (this.cell.isSetV()) { this.cell.unsetV(); } - + } public void setCellNum(int num) { - setCellNum((short)num); + setCellNum((short)num); } public void setCellNum(short num) { checkBounds(num); @@ -303,13 +330,13 @@ public class XSSFCell implements Cell { } public void setCellStyle(CellStyle style) { - if(style == null) { - this.cell.setS(0); - } else { - this.cell.setS( - row.getSheet().getWorkbook().getStylesSource().putStyle(style) - ); - } + if(style == null) { + this.cell.setS(0); + } else { + this.cell.setS( + row.getSheet().getWorkbook().getStylesSource().putStyle(style) + ); + } } public void setCellType(int cellType) { @@ -364,7 +391,7 @@ public class XSSFCell implements Cell { if ((this.cell.getT() != STCellType.B) && (this.cell.getT() != STCellType.STR)) { this.cell.setT(STCellType.B); - } + } this.cell.setV(value ? TRUE_AS_STRING : FALSE_AS_STRING); } @@ -372,12 +399,12 @@ public class XSSFCell implements Cell { public String toString() { return "[" + this.row.getRowNum() + "," + this.getCellNum() + "] " + this.cell.getV(); } - + /** * Returns the raw, underlying ooxml value for the cell */ public String getRawValue() { - return this.cell.getV(); + return this.cell.getV(); } /** @@ -396,14 +423,14 @@ public class XSSFCell implements Cell { /** * Creates an XSSFRichTextString for you. */ - public RichTextString createRichTextString(String text) { - return new XSSFRichTextString(text); - } - - public Hyperlink getHyperlink() { - return row.getSheet().getHyperlink(row.getRowNum(), cellNum); - } - public void setHyperlink(Hyperlink hyperlink) { - row.getSheet().setCellHyperlink((XSSFHyperlink)hyperlink); - } + public RichTextString createRichTextString(String text) { + return new XSSFRichTextString(text); + } + + public Hyperlink getHyperlink() { + return row.getSheet().getHyperlink(row.getRowNum(), cellNum); + } + public void setHyperlink(Hyperlink hyperlink) { + row.getSheet().setCellHyperlink((XSSFHyperlink)hyperlink); + } } diff --git a/src/ooxml/testcases/org/apache/poi/xssf/AllXSSFTests.java b/src/ooxml/testcases/org/apache/poi/xssf/AllXSSFTests.java new file mode 100644 index 000000000..25a524ab0 --- /dev/null +++ b/src/ooxml/testcases/org/apache/poi/xssf/AllXSSFTests.java @@ -0,0 +1,54 @@ +/* ==================================================================== + 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.xssf; + +import junit.framework.Test; +import junit.framework.TestSuite; + +import org.apache.poi.xssf.eventusermodel.TestXSSFReader; +import org.apache.poi.xssf.extractor.TestXSSFExcelExtractor; +import org.apache.poi.xssf.io.TestLoadSaveXSSF; +import org.apache.poi.xssf.model.TestCommentsTable; +import org.apache.poi.xssf.model.TestStylesTable; +import org.apache.poi.xssf.usermodel.AllXSSFUsermodelTests; +import org.apache.poi.xssf.util.TestCTColComparator; +import org.apache.poi.xssf.util.TestCellReference; +import org.apache.poi.xssf.util.TestNumericRanges; + +/** + * Collects all tests for org.apache.poi.xssf and sub-packages. + * + * @author Josh Micich + */ +public final class AllXSSFTests { + + public static Test suite() { + TestSuite result = new TestSuite(AllXSSFTests.class.getName()); + result.addTest(AllXSSFUsermodelTests.suite()); + + result.addTestSuite(TestXSSFReader.class); + result.addTestSuite(TestXSSFExcelExtractor.class); + result.addTestSuite(TestLoadSaveXSSF.class); + result.addTestSuite(TestCommentsTable.class); + result.addTestSuite(TestStylesTable.class); + result.addTestSuite(TestCellReference.class); + result.addTestSuite(TestCTColComparator.class); + result.addTestSuite(TestNumericRanges.class); + return result; + } +} diff --git a/src/ooxml/testcases/org/apache/poi/xssf/usermodel/AllXSSFUsermodelTests.java b/src/ooxml/testcases/org/apache/poi/xssf/usermodel/AllXSSFUsermodelTests.java new file mode 100644 index 000000000..b68104a86 --- /dev/null +++ b/src/ooxml/testcases/org/apache/poi/xssf/usermodel/AllXSSFUsermodelTests.java @@ -0,0 +1,56 @@ +/* ==================================================================== + 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.xssf.usermodel; + +import junit.framework.Test; +import junit.framework.TestSuite; + +import org.apache.poi.xssf.usermodel.extensions.TestXSSFBorder; +import org.apache.poi.xssf.usermodel.extensions.TestXSSFCellFill; +import org.apache.poi.xssf.usermodel.extensions.TestXSSFSheetComments; +import org.apache.poi.xssf.usermodel.helpers.TestColumnHelper; +import org.apache.poi.xssf.usermodel.helpers.TestHeaderFooterHelper; + +/** + * Collects all tests for org.apache.poi.xssf.usermodel and sub-packages. + * + * @author Josh Micich + */ +public final class AllXSSFUsermodelTests { + + public static Test suite() { + TestSuite result = new TestSuite(AllXSSFUsermodelTests.class.getName()); + result.addTestSuite(TestXSSFBorder.class); + result.addTestSuite(TestXSSFCellFill.class); + result.addTestSuite(TestXSSFHeaderFooter.class); + result.addTestSuite(TestXSSFSheetComments.class); + result.addTestSuite(TestColumnHelper.class); + result.addTestSuite(TestHeaderFooterHelper.class); + result.addTestSuite(TestFormulaEvaluatorOnXSSF.class); + result.addTestSuite(TestXSSFCell.class); + result.addTestSuite(TestXSSFCellStyle.class); + result.addTestSuite(TestXSSFComment.class); + result.addTestSuite(TestXSSFDialogSheet.class); + result.addTestSuite(TestXSSFFormulaEvaluation.class); + result.addTestSuite(TestXSSFHeaderFooter.class); + result.addTestSuite(TestXSSFRow.class); + result.addTestSuite(TestXSSFSheet.class); + result.addTestSuite(TestXSSFWorkbook.class); + return result; + } +} diff --git a/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestFormulaEvaluatorOnXSSF.java b/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestFormulaEvaluatorOnXSSF.java index cd8fcfe50..3f84b2ba3 100644 --- a/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestFormulaEvaluatorOnXSSF.java +++ b/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestFormulaEvaluatorOnXSSF.java @@ -41,7 +41,6 @@ import org.openxml4j.opc.Package; * Periodically, you should open FormulaEvalTestData.xls in * Excel 2007, and re-save it as FormulaEvalTestData_Copy.xlsx * - * Currently disabled, as doesn't work */ public final class TestFormulaEvaluatorOnXSSF extends TestCase { @@ -178,7 +177,7 @@ public final class TestFormulaEvaluatorOnXSSF extends TestCase { * Disabled for now, as many things seem to break * for XSSF, which is a shame */ - public void DISABLEDtestFunctionsFromTestSpreadsheet() { + public void testFunctionsFromTestSpreadsheet() { processFunctionGroup(SS.START_OPERATORS_ROW_INDEX, null); processFunctionGroup(SS.START_FUNCTIONS_ROW_INDEX, null); @@ -261,8 +260,19 @@ public final class TestFormulaEvaluatorOnXSSF extends TestCase { if (c == null || c.getCellType() != Cell.CELL_TYPE_FORMULA) { continue; } + if(isIgnoredFormulaTestCase(c.getCellFormula())) { + continue; + } - FormulaEvaluator.CellValue actualValue = evaluator.evaluate(c); + FormulaEvaluator.CellValue actualValue; + try { + actualValue = evaluator.evaluate(c); + } catch (RuntimeException e) { + _evaluationFailureCount ++; + printShortStackTrace(System.err, e); + result = Result.SOME_EVALUATIONS_FAILED; + continue; + } Cell expectedValueCell = getExpectedValueCell(expectedValuesRow, colnum); try { @@ -281,10 +291,29 @@ public final class TestFormulaEvaluatorOnXSSF extends TestCase { return result; } + /* + * TODO - these are all formulas which currently (Apr-2008) break on ooxml + */ + private static boolean isIgnoredFormulaTestCase(String cellFormula) { + if ("COLUMN(1:2)".equals(cellFormula) || "ROW(2:3)".equals(cellFormula)) { + // full row ranges are not parsed properly yet. + // These cases currently work in svn trunk because of another bug which causes the + // formula to get rendered as COLUMN($A$1:$IV$2) or ROW($A$2:$IV$3) + return true; + } + if ("ISREF(currentcell())".equals(cellFormula)) { + // currently throws NPE because unknown function "currentcell" causes name lookup + // Name lookup requires some equivalent object of the Workbook within xSSFWorkbook. + return true; + } + return false; + } + + /** * Useful to keep output concise when expecting many failures to be reported by this test case */ - private static void printShortStackTrace(PrintStream ps, AssertionFailedError e) { + private static void printShortStackTrace(PrintStream ps, Throwable e) { StackTraceElement[] stes = e.getStackTrace(); int startIx = 0; diff --git a/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFRow.java b/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFRow.java index 314d49c51..54ded70ca 100644 --- a/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFRow.java +++ b/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFRow.java @@ -24,43 +24,45 @@ import junit.framework.TestCase; import org.apache.poi.ss.usermodel.Cell; import org.apache.poi.xssf.usermodel.TestXSSFCell.DummySharedStringSource; - -public class TestXSSFRow extends TestCase { +/** + * Tests for XSSFRow + */ +public final class TestXSSFRow extends TestCase { /** * Test adding cells to a row in various places and see if we can find them again. */ public void testAddAndIterateCells() { XSSFRow row = new XSSFRow(createParentObjects()); - + // One cell at the beginning Cell cell1 = row.createCell((short) 1); - Iterator it = row.cellIterator(); + Iterator it = row.cellIterator(); assertTrue(it.hasNext()); assertTrue(cell1 == it.next()); assertFalse(it.hasNext()); - + // Add another cell at the end Cell cell2 = row.createCell((short) 99); - it = row.cellIterator(); + it = row.cellIterator(); assertTrue(it.hasNext()); assertTrue(cell1 == it.next()); assertTrue(it.hasNext()); assertTrue(cell2 == it.next()); - + // Add another cell at the beginning Cell cell3 = row.createCell((short) 0); - it = row.cellIterator(); + it = row.cellIterator(); assertTrue(it.hasNext()); assertTrue(cell3 == it.next()); assertTrue(it.hasNext()); assertTrue(cell1 == it.next()); assertTrue(it.hasNext()); assertTrue(cell2 == it.next()); - + // Replace cell1 Cell cell4 = row.createCell((short) 1); - it = row.cellIterator(); + it = row.cellIterator(); assertTrue(it.hasNext()); assertTrue(cell3 == it.next()); assertTrue(it.hasNext()); @@ -68,7 +70,7 @@ public class TestXSSFRow extends TestCase { assertTrue(it.hasNext()); assertTrue(cell2 == it.next()); assertFalse(it.hasNext()); - + // Add another cell, specifying the cellType Cell cell5 = row.createCell((short) 2, Cell.CELL_TYPE_STRING); it = row.cellIterator(); @@ -83,24 +85,26 @@ public class TestXSSFRow extends TestCase { assertTrue(cell2 == it.next()); assertEquals(Cell.CELL_TYPE_STRING, cell5.getCellType()); } - - public void testGetCell() throws Exception { + + public void testGetCell() { XSSFRow row = getSampleRow(); - + assertNotNull(row.getCell((short) 2)); assertNotNull(row.getCell((short) 3)); assertNotNull(row.getCell((short) 4)); - assertEquals(Cell.CELL_TYPE_NUMERIC, row.getCell((short) 3).getCellType()); + // cell3 may have been created as CELL_TYPE_NUMERIC, but since there is no numeric + // value set yet, its cell type is classified as 'blank' + assertEquals(Cell.CELL_TYPE_BLANK, row.getCell((short) 3).getCellType()); assertNull(row.getCell((short) 5)); } - - public void testGetPhysicalNumberOfCells() throws Exception { + + public void testGetPhysicalNumberOfCells() { XSSFRow row = getSampleRow(); assertEquals(7, row.getPhysicalNumberOfCells()); } - - public void testGetFirstCellNum() throws Exception { - // Test a row with some cells + + public void testGetFirstCellNum() { + // Test a row with some cells XSSFRow row = getSampleRow(); assertFalse(row.getFirstCellNum() == (short) 0); assertEquals((short) 2, row.getFirstCellNum()); @@ -109,13 +113,13 @@ public class TestXSSFRow extends TestCase { Cell cell = row.getCell((short) 2); row.removeCell(cell); assertFalse(row.getFirstCellNum() == (short) 2); - + // Test a row without cells XSSFRow emptyRow = new XSSFRow(createParentObjects()); assertEquals(-1, emptyRow.getFirstCellNum()); } - - public void testLastCellNum() throws Exception { + + public void testLastCellNum() { XSSFRow row = getSampleRow(); assertEquals(100, row.getLastCellNum()); @@ -123,26 +127,25 @@ public class TestXSSFRow extends TestCase { row.removeCell(cell); assertFalse(row.getLastCellNum() == (short) 100); } - - public void testRemoveCell() throws Exception { - XSSFRow row = getSampleRow(); - // Test removing the first cell - Cell firstCell = row.getCell((short) 2); - assertNotNull(firstCell); - assertEquals(7, row.getPhysicalNumberOfCells()); - row.removeCell(firstCell); - assertEquals(6, row.getPhysicalNumberOfCells()); - firstCell = row.getCell((short) 2); - assertNull(firstCell); - - // Test removing the last cell - Cell lastCell = row.getCell((short) 100); - row.removeCell(lastCell); - + public void testRemoveCell() { + XSSFRow row = getSampleRow(); + + // Test removing the first cell + Cell firstCell = row.getCell((short) 2); + assertNotNull(firstCell); + assertEquals(7, row.getPhysicalNumberOfCells()); + row.removeCell(firstCell); + assertEquals(6, row.getPhysicalNumberOfCells()); + firstCell = row.getCell((short) 2); + assertNull(firstCell); + + // Test removing the last cell + Cell lastCell = row.getCell((short) 100); + row.removeCell(lastCell); } - - public void testGetSetHeight() throws Exception { + + public void testGetSetHeight() { XSSFRow row = getSampleRow(); // I assume that "ht" attribute value is in 'points', please verify that // Test that no rowHeight is set @@ -150,37 +153,37 @@ public class TestXSSFRow extends TestCase { // Set a rowHeight in twips (1/20th of a point) and test the new value row.setHeight((short) 240); assertEquals((short) 240, row.getHeight()); - assertEquals((float) 12, row.getHeightInPoints()); + assertEquals(12F, row.getHeightInPoints()); // Set a new rowHeight in points and test the new value - row.setHeightInPoints((float) 13); + row.setHeightInPoints(13F); assertEquals((float) 13, row.getHeightInPoints()); assertEquals((short) 260, row.getHeight()); } - + public void testGetSetZeroHeight() throws Exception { XSSFRow row = getSampleRow(); assertFalse(row.getZeroHeight()); row.setZeroHeight(true); assertTrue(row.getZeroHeight()); } - + /** * Method that returns a row with some sample cells * @return row */ - public XSSFRow getSampleRow() { - XSSFRow row = new XSSFRow(createParentObjects()); - row.createCell((short) 2); - row.createCell((short) 3, Cell.CELL_TYPE_NUMERIC); - row.createCell((short) 4); - row.createCell((short) 6); - row.createCell((short) 7); - row.createCell((short) 8); - row.createCell((short) 100); - return row; + private static XSSFRow getSampleRow() { + XSSFRow row = new XSSFRow(createParentObjects()); + row.createCell((short) 2); + row.createCell((short) 3, Cell.CELL_TYPE_NUMERIC); + row.createCell((short) 4); + row.createCell((short) 6); + row.createCell((short) 7); + row.createCell((short) 8); + row.createCell((short) 100); + return row; } - private XSSFSheet createParentObjects() { + private static XSSFSheet createParentObjects() { XSSFWorkbook wb = new XSSFWorkbook(); wb.setSharedStringSource(new DummySharedStringSource()); return new XSSFSheet(wb);