Fix bug #43008, by deprecating setCellNum() on HSSFCell, and adding moveCell() to HSSFRow, which correctly updates all the references

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

src/documentation/content/xdocs/changes.xml

@@ -36,6 +36,7 @@
 
 		<!-- Don't forget to update status.xml too! -->
         <release version="3.0.2-FINAL" date="2008-??-??">
+            <action dev="POI-DEVELOPERS" type="add">43008 - Add a moveCell method to HSSFRow, and deprecate setCellNum(), which didn't update things properly</action>
             <action dev="POI-DEVELOPERS" type="fix">43058 - Support setting row grouping on files from CR IX, which lack GutsRecords</action>
             <action dev="POI-DEVELOPERS" type="fix">31795 - Support cloning of sheets with certain drawing objects on them</action>
             <action dev="POI-DEVELOPERS" type="fix">43902 - Don't consider merged regions when auto-sizing columns</action>

src/documentation/content/xdocs/status.xml

@@ -33,6 +33,7 @@
 	<!-- Don't forget to update changes.xml too! -->
     <changes>
         <release version="3.0.2-FINAL" date="2008-??-??">
+            <action dev="POI-DEVELOPERS" type="add">43008 - Add a moveCell method to HSSFRow, and deprecate setCellNum(), which didn't update things properly</action>
             <action dev="POI-DEVELOPERS" type="fix">43058 - Support setting row grouping on files from CR IX, which lack GutsRecords</action>
             <action dev="POI-DEVELOPERS" type="fix">31795 - Support cloning of sheets with certain drawing objects on them</action>
             <action dev="POI-DEVELOPERS" type="fix">43902 - Don't consider merged regions when auto-sizing columns</action>

src/java/org/apache/poi/hssf/usermodel/HSSFCell.java

@@ -24,17 +24,33 @@
  */
 package org.apache.poi.hssf.usermodel;
 
+import java.text.DateFormat;
+import java.text.SimpleDateFormat;
+import java.util.Calendar;
+import java.util.Date;
+import java.util.HashMap;
+import java.util.Iterator;
+
 import org.apache.poi.hssf.model.FormulaParser;
 import org.apache.poi.hssf.model.Sheet;
 import org.apache.poi.hssf.model.Workbook;
-import org.apache.poi.hssf.record.*;
+import org.apache.poi.hssf.record.BlankRecord;
+import org.apache.poi.hssf.record.BoolErrRecord;
+import org.apache.poi.hssf.record.CellValueRecordInterface;
+import org.apache.poi.hssf.record.CommonObjectDataSubRecord;
+import org.apache.poi.hssf.record.ExtendedFormatRecord;
+import org.apache.poi.hssf.record.FormulaRecord;
+import org.apache.poi.hssf.record.LabelSSTRecord;
+import org.apache.poi.hssf.record.NoteRecord;
+import org.apache.poi.hssf.record.NumberRecord;
+import org.apache.poi.hssf.record.ObjRecord;
+import org.apache.poi.hssf.record.Record;
+import org.apache.poi.hssf.record.SubRecord;
+import org.apache.poi.hssf.record.TextObjectRecord;
+import org.apache.poi.hssf.record.UnicodeString;
 import org.apache.poi.hssf.record.aggregates.FormulaRecordAggregate;
 import org.apache.poi.hssf.record.formula.Ptg;
 
-import java.text.DateFormat;
-import java.text.SimpleDateFormat;
-import java.util.*;
-
 /**
  * High level representation of a cell in a row of a spreadsheet.
  * Cells can be numeric, formula-based or string-based (text).  The cell type
@@ -266,15 +282,25 @@ public class HSSFCell
     }
 
     /**
-     * set the cell's number within the row (0 based)
+     * Set the cell's number within the row (0 based).
      * @param num  short the cell number
+     * @deprecated Doesn't update the row's idea of what cell this is, use {@link HSSFRow#moveCell(HSSFCell, short)} instead
      */
-
     public void setCellNum(short num)
     {
         record.setColumn(num);
     }
     
+    /**
+     * Updates the cell record's idea of what
+     *  column it belongs in (0 based)
+     * @param num the new cell number
+     */
+    protected void updateCellNum(short num)
+    {
+    	record.setColumn(num);
+    }
+
     /**
      *  get the cell's number within the row
      * @return short reperesenting the column number (logical!)

src/java/org/apache/poi/hssf/usermodel/HSSFRow.java

@@ -22,15 +22,14 @@
  */
 package org.apache.poi.hssf.usermodel;
 
+import java.util.Iterator;
+import java.util.NoSuchElementException;
+
 import org.apache.poi.hssf.model.Sheet;
 import org.apache.poi.hssf.model.Workbook;
 import org.apache.poi.hssf.record.CellValueRecordInterface;
 import org.apache.poi.hssf.record.RowRecord;
 
-import java.util.HashMap;
-import java.util.Iterator;
-import java.util.NoSuchElementException;
-
 /**
  * High level representation of a row of a spreadsheet.
  *
@@ -157,11 +156,15 @@ public class HSSFRow
      * remove the HSSFCell from this row.
      * @param cell to remove
      */
-    public void removeCell(HSSFCell cell)
+    public void removeCell(HSSFCell cell) {
-    {
+    	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)
         {
@@ -234,10 +237,33 @@ public class HSSFRow
     	return row.getOutlineLevel();
     }
     
+    /**
+     * Moves the supplied cell to a new column, which
+     *  must not already have a cell there!
+     * @param cell The cell to move
+     * @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);
+    }
+
     /**
      * used internally to add a cell.
      */
-
     private void addCell(HSSFCell cell)
     {
         short column=cell.getCellNum();

src/testcases/org/apache/poi/hssf/usermodel/TestHSSFRow.java

@@ -102,7 +102,48 @@ public class TestHSSFRow
         file.delete();
         assertEquals(-1, sheet.getRow((short) 0).getLastCellNum());
         assertEquals(-1, sheet.getRow((short) 0).getFirstCellNum());
+    }
     
+    public void testMoveCell() throws Exception {
+        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());
+        
+        // Try to move to somewhere else that's used
+        try {
+        	row.moveCell(cellB2, (short)3);
+        	fail();
+        } catch(IllegalArgumentException e) {}
+        
+        // Try to move one off a different row
+        try {
+        	row.moveCell(cellA2, (short)3);
+        	fail();
+        } catch(IllegalArgumentException e) {}
+        
+        // Move somewhere spare
+        assertNotNull(row.getCell((short)1));
+    	row.moveCell(cellB2, (short)5);
+        assertNull(row.getCell((short)1));
+        assertNotNull(row.getCell((short)5));
+    	
+    	assertEquals(5, cellB2.getCellNum());
+        assertEquals(2, row.getFirstCellNum());
+        assertEquals(5, row.getLastCellNum());
         
     }