[patch] svn diff of changes to HSSFRow and test classes

patch43901-diff.txt (text/plain), 17.52 KB, created by Josh Micich on 2008-03-06 11:42:27 UTC

(hide)

Description: svn diff of changes to HSSFRow and test classes

Filename:

MIME Type:

Creator: Josh Micich

Created: 2008-03-06 11:42:27 UTC

Size: 17.52 KB

patch

obsolete

>Index: C:\josh\source\poi\subv\trunk/src/java/org/apache/poi/hssf/usermodel/HSSFRow.java >=================================================================== >--- C:\josh\source\poi\subv\trunk/src/java/org/apache/poi/hssf/usermodel/HSSFRow.java (revision 634375) >+++ C:\josh\source\poi\subv\trunk/src/java/org/apache/poi/hssf/usermodel/HSSFRow.java (working copy) >@@ -15,11 +15,6 @@ > limitations under the License. > ==================================================================== */ > >-/* >- * HSSFRow.java >- * >- * Created on September 30, 2001, 3:44 PM >- */ > package org.apache.poi.hssf.usermodel; > > import java.util.Iterator; >@@ -38,11 +33,8 @@ > * @author Andrew C. Oliver (acoliver at apache dot org) > * @author Glen Stampoultzis (glens at apache.org) > */ >+public final class HSSFRow implements Comparable { > >-public class HSSFRow >- implements Comparable >-{ >- > // used for collections > public final static int INITIAL_CAPACITY = 5; > //private short rowNum; >@@ -157,26 +149,31 @@ > * @param cell to remove > */ > public void removeCell(HSSFCell cell) { >- removeCell(cell, true); >+ if(cell == null) { >+ throw new IllegalArgumentException("cell must not be null"); >+ } >+ removeCell(cell, true); > } > private void removeCell(HSSFCell cell, boolean alsoRemoveRecords) { >- if(alsoRemoveRecords) { >- CellValueRecordInterface cval = cell.getCellValueRecord(); >- sheet.removeValueRecord(getRowNum(), cval); >- } >- >+ > short column=cell.getCellNum(); >- if(cell!=null && column<cells.length) >- { >- cells[column]=null; >+ if(column < 0) { >+ throw new RuntimeException("Negative cell indexes not allowed"); > } >- >- if (cell.getCellNum() == row.getLastCol()) >- { >- row.setLastCol(findLastCell(row.getLastCol())); >+ if(column >= cells.length || cell != cells[column]) { >+ throw new RuntimeException("Specified cell is not from this row"); > } >- if (cell.getCellNum() == row.getFirstCol()) >- { >+ cells[column]=null; >+ >+ if(alsoRemoveRecords) { >+ CellValueRecordInterface cval = cell.getCellValueRecord(); >+ sheet.removeValueRecord(getRowNum(), cval); >+ } >+ >+ if (cell.getCellNum()+1 == row.getLastCol()) { >+ row.setLastCol((short) (findLastCell(row.getLastCol())+1)); >+ } >+ if (cell.getCellNum() == row.getFirstCol()) { > row.setFirstCol(findFirstCell(row.getFirstCol())); > } > } >@@ -234,7 +231,7 @@ > * TODO - Should this really be public? > */ > protected int getOutlineLevel() { >- return row.getOutlineLevel(); >+ return row.getOutlineLevel(); > } > > /** >@@ -244,55 +241,48 @@ > * @param newColumn The new column number (0 based) > */ > public void moveCell(HSSFCell cell, short newColumn) { >- // Ensure the destination is free >- if(cells.length > newColumn && cells[newColumn] != null) { >- throw new IllegalArgumentException("Asked to move cell to column " + newColumn + " but there's already a cell there"); >- } >- >- // Check it's one of ours >- if(! cells[cell.getCellNum()].equals(cell)) { >- throw new IllegalArgumentException("Asked to move a cell, but it didn't belong to our row"); >- } >- >- // Move the cell to the new position >- // (Don't remove the records though) >- removeCell(cell, false); >- cell.updateCellNum(newColumn); >- addCell(cell); >+ // Ensure the destination is free >+ if(cells.length > newColumn && cells[newColumn] != null) { >+ throw new IllegalArgumentException("Asked to move cell to column " + newColumn + " but there's already a cell there"); >+ } >+ >+ // Check it's one of ours >+ if(! cells[cell.getCellNum()].equals(cell)) { >+ throw new IllegalArgumentException("Asked to move a cell, but it didn't belong to our row"); >+ } >+ >+ // Move the cell to the new position >+ // (Don't remove the records though) >+ removeCell(cell, false); >+ cell.updateCellNum(newColumn); >+ addCell(cell); > } > > /** > * used internally to add a cell. > */ >- private void addCell(HSSFCell cell) >- { >+ private void addCell(HSSFCell cell) { >+ > short column=cell.getCellNum(); >- if (row.getFirstCol() == -1) >- { >- row.setFirstCol(column); >+ // re-allocate cells array as required. >+ if(column>=cells.length) { >+ HSSFCell[] oldCells=cells; >+ int newSize=oldCells.length*2; >+ if(newSize<column+1) { >+ newSize=column+1; >+ } >+ cells=new HSSFCell[newSize]; >+ System.arraycopy(oldCells,0,cells,0,oldCells.length); > } >- if (row.getLastCol() == -1) >- { >- row.setLastCol(column); >- } >- >- if(column>=cells.length) >- { >- HSSFCell[] oldCells=cells; >- int newSize=oldCells.length*2; >- if(newSize<column+1) newSize=column+1; >- cells=new HSSFCell[newSize]; >- System.arraycopy(oldCells,0,cells,0,oldCells.length); >- } > cells[column]=cell; >- >- if (column < row.getFirstCol()) >- { >+ >+ // fix up firstCol and lastCol indexes >+ if (row.getFirstCol() == -1 || column < row.getFirstCol()) { > row.setFirstCol(column); > } >- if (column > row.getLastCol()) >- { >- row.setLastCol(column); >+ >+ if (row.getLastCol() == -1 || column >= row.getLastCol()) { >+ row.setLastCol((short) (column+1)); // +1 -> for one past the last index > } > } > >@@ -324,16 +314,29 @@ > } > > /** >- * gets the number of the last cell contained in this row <b>PLUS ONE</b>. >- * @return short representing the last logical cell in the row <b>PLUS ONE</b>, or -1 if the row does not contain any cells. >+ * Gets the index of the last cell contained in this row <b>PLUS ONE</b>. The result also >+ * happens to be the 1-based column number of the last cell. This value can be used as a >+ * standard upper bound when iterating over cells: >+ * <pre> >+ * short minColIx = row.getFirstCellNum(); >+ * short maxColIx = row.getLastCellNum(); >+ * for(short colIx=minColIx; colIx&lt;maxColIx; colIx++) { >+ * HSSFCell cell = row.getCell(colIx); >+ * if(cell == null) { >+ * continue; >+ * } >+ * //... do something with cell >+ * } >+ * </pre> >+ * >+ * @return short representing the last logical cell in the row <b>PLUS ONE</b>, or -1 if the >+ * row does not contain any cells. > */ >- >- public short getLastCellNum() >- { >- if (getPhysicalNumberOfCells() == 0) >+ public short getLastCellNum() { >+ if (getPhysicalNumberOfCells() == 0) { > return -1; >- else >- return row.getLastCol(); >+ } >+ return row.getLastCol(); > } > > >@@ -493,8 +496,8 @@ > } > > public Object next() { >- if (!hasNext()) >- throw new NoSuchElementException("At last element"); >+ if (!hasNext()) >+ throw new NoSuchElementException("At last element"); > HSSFCell cell=cells[nextId]; > thisId=nextId; > findNext(); >@@ -502,8 +505,8 @@ > } > > public void remove() { >- if (thisId == -1) >- throw new IllegalStateException("remove() called before next()"); >+ if (thisId == -1) >+ throw new IllegalStateException("remove() called before next()"); > cells[thisId]=null; > } > >Index: C:\josh\source\poi\subv\trunk/src/testcases/org/apache/poi/hssf/usermodel/TestBugs.java >=================================================================== >--- C:\josh\source\poi\subv\trunk/src/testcases/org/apache/poi/hssf/usermodel/TestBugs.java (revision 634375) >+++ C:\josh\source\poi\subv\trunk/src/testcases/org/apache/poi/hssf/usermodel/TestBugs.java (working copy) >@@ -1104,29 +1104,6 @@ > > assertEquals(1, wb.getNumberOfSheets()); > } >- >- /** >- * POI is producing files with the wrong last-column >- * number on the row >- */ >- public void BROKENtest43901() { >- HSSFWorkbook book = new HSSFWorkbook(); >- HSSFSheet sheet = book.createSheet("test"); >- >- // New row has last col -1 >- HSSFRow row = sheet.createRow(0); >- assertEquals(-1, row.getLastCellNum()); >- if(row.getLastCellNum() == 0) { >- fail("Identified bug 43901"); >- } >- >- // Create two cells, will return one higher >- // than that for the last number >- row.createCell((short) 0); >- assertEquals(1, row.getLastCellNum()); >- row.createCell((short) 255); >- assertEquals(256, row.getLastCellNum()); >- } > } > > >Index: C:\josh\source\poi\subv\trunk/src/testcases/org/apache/poi/hssf/usermodel/TestHSSFRow.java >=================================================================== >--- C:\josh\source\poi\subv\trunk/src/testcases/org/apache/poi/hssf/usermodel/TestHSSFRow.java (revision 634375) >+++ C:\josh\source\poi\subv\trunk/src/testcases/org/apache/poi/hssf/usermodel/TestHSSFRow.java (working copy) >@@ -1,4 +1,3 @@ >- > /* ==================================================================== > Licensed to the Apache Software Foundation (ASF) under one or more > contributor license agreements. See the NOTICE file distributed with >@@ -15,34 +14,22 @@ > See the License for the specific language governing permissions and > limitations under the License. > ==================================================================== */ >- > > package org.apache.poi.hssf.usermodel; > >+import java.io.ByteArrayInputStream; >+import java.io.ByteArrayOutputStream; >+ > import junit.framework.TestCase; > >-import java.io.File; >-import java.io.FileInputStream; >-import java.io.FileOutputStream; >- >-import org.apache.poi.util.TempFile; >- > /** > * Test HSSFRow is okay. > * > * @author Glen Stampoultzis (glens at apache.org) > */ >-public class TestHSSFRow >- extends TestCase >-{ >- public TestHSSFRow(String s) >- { >- super(s); >- } >+public final class TestHSSFRow extends TestCase { > >- public void testLastAndFirstColumns() >- throws Exception >- { >+ public void testLastAndFirstColumns() { > HSSFWorkbook workbook = new HSSFWorkbook(); > HSSFSheet sheet = workbook.createSheet(); > HSSFRow row = sheet.createRow((short) 0); >@@ -51,127 +38,146 @@ > > row.createCell((short) 2); > assertEquals(2, row.getFirstCellNum()); >- assertEquals(2, row.getLastCellNum()); >+ assertEquals(3, row.getLastCellNum()); > > row.createCell((short) 1); > assertEquals(1, row.getFirstCellNum()); >- assertEquals(2, row.getLastCellNum()); >- >+ assertEquals(3, row.getLastCellNum()); >+ > // check the exact case reported in 'bug' 43901 - notice that the cellNum is '0' based > row.createCell((short) 3); > assertEquals(1, row.getFirstCellNum()); >- assertEquals(3, row.getLastCellNum()); >- >+ assertEquals(4, row.getLastCellNum()); > } > >- public void testRemoveCell() >- throws Exception >- { >+ public void testRemoveCell() throws Exception { > HSSFWorkbook workbook = new HSSFWorkbook(); > HSSFSheet sheet = workbook.createSheet(); > HSSFRow row = sheet.createRow((short) 0); > assertEquals(-1, row.getLastCellNum()); > assertEquals(-1, row.getFirstCellNum()); > row.createCell((short) 1); >- assertEquals(1, row.getLastCellNum()); >+ assertEquals(2, row.getLastCellNum()); > assertEquals(1, row.getFirstCellNum()); > row.createCell((short) 3); >- assertEquals(3, row.getLastCellNum()); >+ assertEquals(4, row.getLastCellNum()); > assertEquals(1, row.getFirstCellNum()); > row.removeCell(row.getCell((short) 3)); >- assertEquals(1, row.getLastCellNum()); >+ assertEquals(2, row.getLastCellNum()); > assertEquals(1, row.getFirstCellNum()); > row.removeCell(row.getCell((short) 1)); > assertEquals(-1, row.getLastCellNum()); > assertEquals(-1, row.getFirstCellNum()); > >- // check the row record actually writes it out as 0's >+ // all cells on this row have been removed >+ // so check the row record actually writes it out as 0's > byte[] data = new byte[100]; > row.getRowRecord().serialize(0, data); > assertEquals(0, data[6]); > assertEquals(0, data[8]); > >- File file = TempFile.createTempFile("XXX", "XLS"); >- FileOutputStream stream = new FileOutputStream(file); >- workbook.write(stream); >- stream.close(); >- FileInputStream inputStream = new FileInputStream(file); >+ ByteArrayOutputStream baos = new ByteArrayOutputStream(); >+ workbook.write(baos); >+ baos.close(); >+ ByteArrayInputStream inputStream = new ByteArrayInputStream(baos.toByteArray()); > workbook = new HSSFWorkbook(inputStream); > sheet = workbook.getSheetAt(0); >- stream.close(); >- file.delete(); >+ inputStream.close(); >+ > assertEquals(-1, sheet.getRow((short) 0).getLastCellNum()); > assertEquals(-1, sheet.getRow((short) 0).getFirstCellNum()); > } >- >- public void testMoveCell() throws Exception { >+ >+ public void testMoveCell() { > HSSFWorkbook workbook = new HSSFWorkbook(); > HSSFSheet sheet = workbook.createSheet(); > HSSFRow row = sheet.createRow((short) 0); > HSSFRow rowB = sheet.createRow((short) 1); >- >+ > HSSFCell cellA2 = rowB.createCell((short)0); > assertEquals(0, rowB.getFirstCellNum()); > assertEquals(0, rowB.getFirstCellNum()); >- >+ > assertEquals(-1, row.getLastCellNum()); > assertEquals(-1, row.getFirstCellNum()); > HSSFCell cellB2 = row.createCell((short) 1); > HSSFCell cellB3 = row.createCell((short) 2); > HSSFCell cellB4 = row.createCell((short) 3); >- >+ > assertEquals(1, row.getFirstCellNum()); >- assertEquals(3, row.getLastCellNum()); >- >+ assertEquals(4, row.getLastCellNum()); >+ > // Try to move to somewhere else that's used > try { >- row.moveCell(cellB2, (short)3); >- fail(); >- } catch(IllegalArgumentException e) {} >- >+ row.moveCell(cellB2, (short)3); >+ fail("IllegalArgumentException should have been thrown"); >+ } catch(IllegalArgumentException e) { >+ // expected during successful test >+ } >+ > // Try to move one off a different row > try { >- row.moveCell(cellA2, (short)3); >- fail(); >- } catch(IllegalArgumentException e) {} >- >+ row.moveCell(cellA2, (short)3); >+ fail("IllegalArgumentException should have been thrown"); >+ } catch(IllegalArgumentException e) { >+ // expected during successful test >+ } >+ > // Move somewhere spare > assertNotNull(row.getCell((short)1)); >- row.moveCell(cellB2, (short)5); >+ row.moveCell(cellB2, (short)5); > assertNull(row.getCell((short)1)); > assertNotNull(row.getCell((short)5)); >- >- assertEquals(5, cellB2.getCellNum()); >+ >+ assertEquals(5, cellB2.getCellNum()); > assertEquals(2, row.getFirstCellNum()); >- assertEquals(5, row.getLastCellNum()); >- >+ assertEquals(6, row.getLastCellNum()); > } >- >- public void testRowBounds() >- throws Exception >- { >+ >+ public void testRowBounds() { > HSSFWorkbook workbook = new HSSFWorkbook(); > HSSFSheet sheet = workbook.createSheet(); > //Test low row bound >- HSSFRow row = sheet.createRow( (short) 0); >- //Test low row bound exception >- boolean caughtException = false; >+ sheet.createRow( (short) 0); >+ //Test low row bound exception > try { >- row = sheet.createRow(-1); >+ sheet.createRow(-1); >+ fail("IndexOutOfBoundsException should have been thrown"); > } catch (IndexOutOfBoundsException ex) { >- caughtException = true; >- } >- assertTrue(caughtException); >- //Test high row bound >- row = sheet.createRow(65535); >- //Test high row bound exception >- caughtException = false; >+ // expected during successful test >+ } >+ >+ //Test high row bound >+ sheet.createRow(65535); >+ //Test high row bound exception > try { >- row = sheet.createRow(65536); >+ sheet.createRow(65536); >+ fail("IndexOutOfBoundsException should have been thrown"); > } catch (IndexOutOfBoundsException ex) { >- caughtException = true; >- } >- assertTrue(caughtException); >+ // expected during successful test >+ } > } >- >+ >+ /** >+ * Prior to patch 43901, POI was producing files with the wrong last-column >+ * number on the row >+ */ >+ public void testLastCellNumIsCorrectAfterAddCell_bug43901(){ >+ HSSFWorkbook book = new HSSFWorkbook(); >+ HSSFSheet sheet = book.createSheet("test"); >+ HSSFRow row = sheet.createRow(0); >+ >+ // New row has last col -1 >+ assertEquals(-1, row.getLastCellNum()); >+ if(row.getLastCellNum() == 0) { >+ fail("Identified bug 43901"); >+ } >+ >+ // Create two cells, will return one higher >+ // than that for the last number >+ row.createCell((short) 0); >+ assertEquals(1, row.getLastCellNum()); >+ row.createCell((short) 255); >+ assertEquals(256, row.getLastCellNum()); >+ } > } View Attachment As Diff View Attachment As Raw

This is ASF Bugzilla: the Apache Software Foundation bug system. In case of problems with the functioning of ASF Bugzilla, please contact bugzilla-admin@apache.org. Please Note: this e-mail address is only for reporting problems with ASF Bugzilla. Mail about any other subject will be silently ignored.

• • Home
  • | New
  • | Browse
  • | Search
  • |
    [?]
  • | Reports
  • | Help
  • | New Account
  • | Log In
    Remember [x]
  • | Forgot Password
    Login: [x]