changed serialize method on Sheet to visitContainedRecords to simplify serialization logic and also allow test code to inspect generated sheet records more directly

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

src/java/org/apache/poi/hssf/model/Sheet.java

@@ -68,6 +68,7 @@ import org.apache.poi.hssf.record.aggregates.MergedCellsTable;
 import org.apache.poi.hssf.record.aggregates.PageSettingsBlock;
 import org.apache.poi.hssf.record.aggregates.RecordAggregate;
 import org.apache.poi.hssf.record.aggregates.RowRecordsAggregate;
+import org.apache.poi.hssf.record.aggregates.RecordAggregate.PositionTrackingVisitor;
 import org.apache.poi.hssf.record.aggregates.RecordAggregate.RecordVisitor;
 import org.apache.poi.hssf.util.CellRangeAddress;
 import org.apache.poi.hssf.util.PaneInformation;
@@ -105,7 +106,6 @@ public final class Sheet implements Model {
     private static POILogger            log              = POILogFactory.getLogger(Sheet.class);
 
     protected ArrayList                  records           =     null;
-              int                        preoffset         =     0;            // offset of the sheet in a new file
     protected int                        dimsloc           =     -1;  // TODO - remove dimsloc
     protected PrintGridlinesRecord       printGridlines    =     null;
     protected GridsetRecord              gridset           =     null;
@@ -148,7 +148,7 @@ public final class Sheet implements Model {
      * @see #createSheet(List,int,int)
      */
     public Sheet() {
-    	_mergedCellsTable = new MergedCellsTable();
+        _mergedCellsTable = new MergedCellsTable();
     }
 
     /**
@@ -245,12 +245,17 @@ public final class Sheet implements Model {
             }
             
             if (rec.getSid() == MergeCellsRecord.sid) {
-            	// when the MergedCellsTable is found in the right place, we expect those records to be contiguous
+                // when the MergedCellsTable is found in the right place, we expect those records to be contiguous
                 RecordStream rs = new RecordStream(inRecs, k);
                 retval._mergedCellsTable.read(rs);
                 k += rs.getCountRead()-1;
                 continue;
             }
+            if (rec.getSid() == UncalcedRecord.sid) {
+                // don't add UncalcedRecord to the list
+                retval._isUncalced = true; // this flag is enough
+                continue;
+            }
 
             if (rec.getSid() == BOFRecord.sid)
             {
@@ -269,9 +274,6 @@ public final class Sheet implements Model {
                     break;
                 }
             }
-            else if (rec.getSid() == UncalcedRecord.sid) {
-                retval._isUncalced = true;
-            }
             else if (rec.getSid() == DimensionsRecord.sid)
             {
                 // Make a columns aggregate if one hasn't ready been created.
@@ -585,61 +587,23 @@ public final class Sheet implements Model {
             log.log(POILogger.DEBUG, "Sheet.setDimensions exiting");
     }
 
-    /**
+    public void visitContainedRecords(RecordVisitor rv, int offset) {
-     * Set the preoffset when using DBCELL records (currently unused) - this is
-     * the position of this sheet within the whole file.
-     *
-     * @param offset the offset of the sheet's BOF within the file.
-     */
 
-    public void setPreOffset(int offset)
+        PositionTrackingVisitor ptv = new PositionTrackingVisitor(rv, offset);
-    {
-        this.preoffset = offset;
-    }
         
-    /**
-     * get the preoffset when using DBCELL records (currently unused) - this is
-     * the position of this sheet within the whole file.
-     *
-     * @return offset the offset of the sheet's BOF within the file.
-     */
-
-    public int getPreOffset()
-    {
-        return preoffset;
-    }
-
-    /**
-     * Serializes all records in the sheet into one big byte array.  Use this to write
-     * the sheet out.
-     *
-     * @param offset to begin write at
-     * @param data   array containing the binary representation of the records in this sheet
-     *
-     */
-
-    public int serialize(int offset, byte [] data)
-    {
-        if (log.check( POILogger.DEBUG ))
-            log.log(POILogger.DEBUG, "Sheet.serialize using offsets");
-
-        int pos       = offset;
         boolean haveSerializedIndex = false;
 
         for (int k = 0; k < records.size(); k++)
         {
             RecordBase record = (RecordBase) records.get(k);
 
-            // Don't write out UncalcedRecord entries, as
+            if (record instanceof RecordAggregate) {
-            //  we handle those specially just below
+                RecordAggregate agg = (RecordAggregate) record;
-            if (record instanceof UncalcedRecord) {
+                agg.visitContainedRecords(ptv);
-                continue;
+            } else {
+                ptv.visitRecord((Record) record);
             }
 
-            // Once the rows have been found in the list of records, start
-            //  writing out the blocked row information. This includes the DBCell references
-            pos += record.serialize(pos, data);
-
             // If the BOF record was just serialized then add the IndexRecord
             if (record instanceof BOFRecord) {
               if (!haveSerializedIndex) {
@@ -649,50 +613,42 @@ public final class Sheet implements Model {
                 // If there are diagrams, they have their own BOFRecords,
                 //  and one shouldn't go in after that!
                 if (_isUncalced) {
-                    UncalcedRecord rec = new UncalcedRecord();
+                    ptv.visitRecord(new UncalcedRecord());
-                    pos += rec.serialize(pos, data);
                 }
                 //Can there be more than one BOF for a sheet? If not then we can
                 //remove this guard. So be safe it is left here.
                 if (_rowsAggregate != null) {
-                  pos += serializeIndexRecord(k, pos, data);
+                	// find forward distance to first RowRecord
+                    int initRecsSize = getSizeOfInitialSheetRecords(k);
+                    int currentPos = ptv.getPosition();
+                    ptv.visitRecord(_rowsAggregate.createIndexRecord(currentPos, initRecsSize));
                 }
               }
             }
         }
-        if (log.check( POILogger.DEBUG )) {
-            log.log(POILogger.DEBUG, "Sheet.serialize returning ");
-        }
-        return pos-offset;
     }
-
     /**
-     * @param indexRecordOffset also happens to be the end of the BOF record
+     * 'initial sheet records' are between INDEX and the 'Row Blocks'
-     * @return the size of the serialized INDEX record
+     * @param bofRecordIndex index of record after which INDEX record is to be placed
+     * @return count of bytes from end of INDEX record to first ROW record.
      */
-    private int serializeIndexRecord(int bofRecordIndex, int indexRecordOffset, byte[] data) {
+    private int getSizeOfInitialSheetRecords(int bofRecordIndex) {
 
-        // 'initial sheet records' are between INDEX and first ROW record.
+        int result = 0;
-        int sizeOfInitialSheetRecords = 0;
         // start just after BOF record (INDEX is not present in this list)
         for (int j = bofRecordIndex + 1; j < records.size(); j++) {
-            RecordBase tmpRec = ((RecordBase) records.get(j));
+            RecordBase tmpRec = (RecordBase) records.get(j);
-            if (tmpRec instanceof UncalcedRecord) {
-                continue;
-            }
             if (tmpRec instanceof RowRecordsAggregate) {
                 break;
             }
-            sizeOfInitialSheetRecords += tmpRec.getRecordSize();
+            result += tmpRec.getRecordSize();
         }
         if (_isUncalced) {
-            sizeOfInitialSheetRecords += UncalcedRecord.getStaticRecordSize();
+            result += UncalcedRecord.getStaticRecordSize();
         }
-        IndexRecord index = _rowsAggregate.createIndexRecord(indexRecordOffset, sizeOfInitialSheetRecords);
+        return result;
-        return index.serialize(indexRecordOffset, data);
     }
 
-
     /**
      * Create a row record.  (does not add it to the records contained in this sheet)
      */
@@ -1351,32 +1307,6 @@ public final class Sheet implements Model {
         }
     }
 
-    /**
-     * @return the serialized size of this sheet
-     */
-    public int getSize() {
-        int retval = 0;
-
-        for ( int k = 0; k < records.size(); k++) {
-            RecordBase record = (RecordBase) records.get(k);
-            if (record instanceof UncalcedRecord) {
-                // skip the UncalcedRecord if present, it's only encoded if the isUncalced flag is set
-                continue;
-            }
-            retval += record.getRecordSize();
-        }
-        // add space for IndexRecord if needed
-        if (_rowsAggregate != null) {
-            // rowsAggregate knows how to make the index record
-            retval += IndexRecord.getRecordSizeForBlockCount(_rowsAggregate.getRowBlockCount());
-        }
-        // Add space for UncalcedRecord
-        if (_isUncalced) {
-            retval += UncalcedRecord.getStaticRecordSize();
-        }
-        return retval;
-    }
-
     public List getRecords()
     {
         return records;

src/java/org/apache/poi/hssf/record/aggregates/RecordAggregate.java

@@ -36,10 +36,16 @@ public abstract class RecordAggregate extends RecordBase {
 	protected final void fillFields(RecordInputStream in) {
 		throw new RuntimeException("Should not be called");
 	}
-    public final short getSid() {
+	public final short getSid() {
 		throw new RuntimeException("Should not be called");
-    }
+	}
 
+	/**
+	 * Visit each of the atomic BIFF records contained in this {@link RecordAggregate} in the order
+	 * that they should be written to file.  Implementors may or may not return the actual 
+	 * {@link Record}s being used to manage POI's internal implementation.  Callers should not
+	 * assume either way, and therefore only attempt to modify those {@link Record}s after cloning
+	 */
 	public abstract void visitContainedRecords(RecordVisitor rv);
 	
 	public final int serialize(int offset, byte[] data) {
@@ -94,4 +100,27 @@ public abstract class RecordAggregate extends RecordBase {
 			_totalSize += r.getRecordSize();
 		}
 	}
+	/**
+	 * A wrapper for {@link RecordVisitor} which accumulates the sizes of all
+	 * records visited.
+	 */
+	public static final class PositionTrackingVisitor implements RecordVisitor {
+		private final RecordVisitor _rv;
+		private int _position;
+
+		public PositionTrackingVisitor(RecordVisitor rv, int initialPosition) {
+			_rv = rv;
+			_position = initialPosition;
+		}
+		public void visitRecord(Record r) {
+			_position += r.getRecordSize();
+			_rv.visitRecord(r);
+		}
+		public void setPosition(int position) {
+			_position = position;
+		}
+		public int getPosition() {
+			return _position;
+		}
+	}
 }

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

@@ -51,6 +51,7 @@ import org.apache.poi.hssf.record.RecordFactory;
 import org.apache.poi.hssf.record.SSTRecord;
 import org.apache.poi.hssf.record.UnicodeString;
 import org.apache.poi.hssf.record.UnknownRecord;
+import org.apache.poi.hssf.record.aggregates.RecordAggregate.RecordVisitor;
 import org.apache.poi.hssf.record.formula.Area3DPtg;
 import org.apache.poi.hssf.record.formula.MemFuncPtg;
 import org.apache.poi.hssf.record.formula.NameXPtg;
@@ -1173,6 +1174,37 @@ public class HSSFWorkbook extends POIDocument
         //poifs.writeFilesystem(stream);
     }
 
+    /**
+     * Totals the sizes of all sheet records and eventually serializes them
+     */
+    private static final class SheetRecordCollector implements RecordVisitor {
+
+        private List _list;
+        private int _totalSize;
+
+        public SheetRecordCollector() {
+            _totalSize = 0;
+            _list = new ArrayList(128);
+        }
+        public int getTotalSize() {
+            return _totalSize;
+        }
+        public void visitRecord(Record r) {
+            _list.add(r);
+            _totalSize+=r.getRecordSize();
+        }
+        public int serialize(int offset, byte[] data) {
+            int result = 0;
+            int nRecs = _list.size();
+            for(int i=0; i<nRecs; i++) {
+                Record rec = (Record)_list.get(i);
+                result += rec.serialize(offset + result, data);
+            }
+            return result;
+        }
+    }
+
+
     /**
      * Method getBytes - get the bytes of just the HSSF portions of the XLS file.
      * Use this to construct a POI POIFSFileSystem yourself.
@@ -1184,9 +1216,7 @@ public class HSSFWorkbook extends POIDocument
      * @see org.apache.poi.hssf.model.Workbook
      * @see org.apache.poi.hssf.model.Sheet
      */
-
+    public byte[] getBytes() {
-    public byte[] getBytes()
-    {
         if (log.check( POILogger.DEBUG )) {
             log.log(DEBUG, "HSSFWorkbook.getBytes()");
         }
@@ -1203,26 +1233,27 @@ public class HSSFWorkbook extends POIDocument
         int totalsize = workbook.getSize();
 
         // pre-calculate all the sheet sizes and set BOF indexes
-        int[] estimatedSheetSizes = new int[nSheets];
+        SheetRecordCollector[] srCollectors = new SheetRecordCollector[nSheets];
         for (int k = 0; k < nSheets; k++) {
             workbook.setSheetBof(k, totalsize);
-            int sheetSize = sheets[k].getSheet().getSize();
+            SheetRecordCollector src = new SheetRecordCollector();
-            estimatedSheetSizes[k] = sheetSize;
+            sheets[k].getSheet().visitContainedRecords(src, totalsize);
-            totalsize += sheetSize;
+            totalsize += src.getTotalSize();
+            srCollectors[k] = src;
         }
 
-
         byte[] retval = new byte[totalsize];
         int pos = workbook.serialize(0, retval);
 
         for (int k = 0; k < nSheets; k++) {
-            int serializedSize = sheets[k].getSheet().serialize(pos, retval);
+            SheetRecordCollector src = srCollectors[k];
-            if (serializedSize != estimatedSheetSizes[k]) {
+            int serializedSize = src.serialize(pos, retval);
+            if (serializedSize != src.getTotalSize()) {
                 // Wrong offset values have been passed in the call to setSheetBof() above.
                 // For books with more than one sheet, this discrepancy would cause excel
                 // to report errors and loose data while reading the workbook
                 throw new IllegalStateException("Actual serialized sheet size (" + serializedSize
-                        + ") differs from pre-calculated size (" + estimatedSheetSizes[k] 
+                        + ") differs from pre-calculated size (" + src.getTotalSize()
                         + ") for sheet (" + k + ")");
                 // TODO - add similar sanity check to ensure that Sheet.serializeIndexRecord() does not write mis-aligned offsets either
             }
@@ -1664,8 +1695,8 @@ public class HSSFWorkbook extends POIDocument
      * Note - This method should only used by POI internally.
      * It may get deleted or change definition in future POI versions
      */
-	public NameXPtg getNameXPtg(String name) {
+    public NameXPtg getNameXPtg(String name) {
-		return workbook.getNameXPtg(name);		
+        return workbook.getNameXPtg(name);
-	}
+    }
 
 }

src/testcases/org/apache/poi/hssf/model/TestSheet.java

@@ -17,7 +17,6 @@
 
 package org.apache.poi.hssf.model;
 
-import java.io.ByteArrayInputStream;
 import java.util.ArrayList;
 import java.util.List;
 
@@ -25,8 +24,6 @@ import junit.framework.AssertionFailedError;
 import junit.framework.TestCase;
 
 import org.apache.poi.hssf.HSSFTestDataSamples;
-import org.apache.poi.hssf.eventmodel.ERFListener;
-import org.apache.poi.hssf.eventmodel.EventRecordFactory;
 import org.apache.poi.hssf.record.BOFRecord;
 import org.apache.poi.hssf.record.BlankRecord;
 import org.apache.poi.hssf.record.CellValueRecordInterface;
@@ -46,6 +43,7 @@ import org.apache.poi.hssf.record.aggregates.ColumnInfoRecordsAggregate;
 import org.apache.poi.hssf.record.aggregates.MergedCellsTable;
 import org.apache.poi.hssf.record.aggregates.PageSettingsBlock;
 import org.apache.poi.hssf.record.aggregates.RowRecordsAggregate;
+import org.apache.poi.hssf.record.aggregates.RecordAggregate.RecordVisitor;
 import org.apache.poi.hssf.usermodel.HSSFCell;
 import org.apache.poi.hssf.usermodel.HSSFRow;
 import org.apache.poi.hssf.usermodel.HSSFSheet;
@@ -88,15 +86,16 @@ public final class TestSheet extends TestCase {
         return result;
     }
 
-    private static final class MergedCellListener implements ERFListener {
+    private static final class MergedCellListener implements RecordVisitor {
 
         private int _count;
         public MergedCellListener() {
             _count = 0;
         }
-        public boolean processRecord(Record rec) {
+        public void visitRecord(Record r) {
-            _count++;
+            if (r instanceof MergeCellsRecord) {
-            return true;
+                _count++;
+            }
         }
         public int getCount() {
             return _count;
@@ -118,12 +117,8 @@ public final class TestSheet extends TestCase {
         assertTrue(sheet.getNumMergedRegions() == regionsToAdd);
 
         //test that the regions were spread out over the appropriate number of records
-        byte[] sheetData = new byte[sheet.getSize()];
-        sheet.serialize(0, sheetData);
         MergedCellListener mcListener = new MergedCellListener();
-        EventRecordFactory erf = new EventRecordFactory(mcListener, new short[] { MergeCellsRecord.sid, });
+        sheet.visitContainedRecords(mcListener, 0);
-//        POIFSFileSystem poifs = new POIFSFileSystem(new ByteArrayInputStream(sheetData));
-        erf.processRecords(new ByteArrayInputStream(sheetData));
         int recordsAdded    = mcListener.getCount();
         int recordsExpected = regionsToAdd/1027;
         if ((regionsToAdd % 1027) != 0)
@@ -416,6 +411,27 @@ public final class TestSheet extends TestCase {
         assertEquals(DEFAULT_IDX, xfindex);
     }
 
+    private static final class SizeCheckingRecordVisitor implements RecordVisitor {
+
+        private int _totalSize;
+        public SizeCheckingRecordVisitor() {
+            _totalSize = 0;
+        }
+        public void visitRecord(Record r) {
+
+            int estimatedSize=r.getRecordSize();
+            byte[] buf = new byte[estimatedSize];
+            int serializedSize = r.serialize(0, buf);
+            if (estimatedSize != serializedSize) {
+                throw new AssertionFailedError("serialized size mismatch for record (" 
+                        + r.getClass().getName() + ")");
+            }
+            _totalSize += estimatedSize;
+        }
+        public int getTotalSize() {
+            return _totalSize;
+        }
+    }
     /**
      * Prior to bug 45066, POI would get the estimated sheet size wrong
      * when an <tt>UncalcedRecord</tt> was present.<p/>
@@ -430,12 +446,12 @@ public final class TestSheet extends TestCase {
         records.add(EOFRecord.instance);
         Sheet sheet = Sheet.createSheet(records, 0, 0);
         
-        int estimatedSize = sheet.getSize();
+        // The original bug was due to different logic for collecting records for sizing and 
-        int serializedSize = sheet.serialize(0, new byte[estimatedSize]);
+        // serialization. The code has since been refactored into a single method for visiting
-        if (serializedSize != estimatedSize) {
+        // all contained records.  Now this test is much less interesting
-            throw new AssertionFailedError("Identified bug 45066 b");
+        SizeCheckingRecordVisitor scrv = new SizeCheckingRecordVisitor();
-        }
+        sheet.visitContainedRecords(scrv, 0);
-        assertEquals(90, serializedSize);
+        assertEquals(90, scrv.getTotalSize());
     }
 
     /**
@@ -479,31 +495,31 @@ public final class TestSheet extends TestCase {
      * That value is found on the IndexRecord.
      */
     private static int getDbCellRecordPos(Sheet sheet) {
-        int size = sheet.getSize();
-        byte[] data = new byte[size];
-        sheet.serialize(0, data);
 
         MyIndexRecordListener myIndexListener = new MyIndexRecordListener();
-        EventRecordFactory erf = new EventRecordFactory(myIndexListener, new short[] { IndexRecord.sid, });
+        sheet.visitContainedRecords(myIndexListener, 0);
-        erf.processRecords(new ByteArrayInputStream(data));
         IndexRecord indexRecord = myIndexListener.getIndexRecord();
         int dbCellRecordPos = indexRecord.getDbcellAt(0);
         return dbCellRecordPos;
     }
 
-    private static final class MyIndexRecordListener implements ERFListener {
+    private static final class MyIndexRecordListener implements RecordVisitor {
 
         private IndexRecord _indexRecord;
         public MyIndexRecordListener() {
             // no-arg constructor
         }
-        public boolean processRecord(Record rec) {
-            _indexRecord = (IndexRecord)rec;
-            return true;
-        }
         public IndexRecord getIndexRecord() {
             return _indexRecord;
         }
+        public void visitRecord(Record r) {
+            if (r instanceof IndexRecord) {
+                if (_indexRecord != null) {
+                    throw new RuntimeException("too many index records");
+                }
+                _indexRecord = (IndexRecord)r;
+            }
+        }
     }
     
     /**

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

@@ -17,23 +17,19 @@
 
 package org.apache.poi.hssf.usermodel;
 
-import java.io.ByteArrayInputStream;
 import java.io.File;
 import java.io.FileInputStream;
 import java.io.FileOutputStream;
 import java.io.IOException;
-import java.util.Iterator;
 
 import junit.framework.TestCase;
 
 import org.apache.poi.hssf.HSSFTestDataSamples;
-import org.apache.poi.hssf.eventmodel.ERFListener;
-import org.apache.poi.hssf.eventmodel.EventRecordFactory;
 import org.apache.poi.hssf.model.Workbook;
 import org.apache.poi.hssf.record.BackupRecord;
 import org.apache.poi.hssf.record.LabelSSTRecord;
 import org.apache.poi.hssf.record.Record;
-import org.apache.poi.hssf.record.aggregates.RowRecordsAggregate;
+import org.apache.poi.hssf.record.aggregates.RecordAggregate.RecordVisitor;
 import org.apache.poi.hssf.util.CellRangeAddress;
 import org.apache.poi.poifs.filesystem.POIFSFileSystem;
 import org.apache.poi.util.TempFile;
@@ -72,10 +68,7 @@ public final class TestWorkbook extends TestCase {
      *             HSSFSheet last row or first row is incorrect.             <P>
      *
      */
-
+    public void testWriteSheetSimple() throws IOException {
-    public void testWriteSheetSimple()
-        throws IOException
-    {
         File             file = TempFile.createTempFile("testWriteSheetSimple",
                                                     ".xls");
         FileOutputStream out  = new FileOutputStream(file);
@@ -84,13 +77,10 @@ public final class TestWorkbook extends TestCase {
         HSSFRow          r    = null;
         HSSFCell         c    = null;
 
-        for (short rownum = ( short ) 0; rownum < 100; rownum++)
+        for (int rownum = 0; rownum < 100; rownum++) {
-        {
             r = s.createRow(rownum);
 
-            // r.setRowNum(( short ) rownum);
+            for (int cellnum = 0; cellnum < 50; cellnum += 2) {
-            for (short cellnum = ( short ) 0; cellnum < 50; cellnum += 2)
-            {
                 c = r.createCell(cellnum);
                 c.setCellValue(rownum * 10000 + cellnum
                                + ((( double ) rownum / 1000)
@@ -104,8 +94,6 @@ public final class TestWorkbook extends TestCase {
         sanityChecker.checkHSSFWorkbook(wb);
         assertEquals("LAST ROW == 99", 99, s.getLastRowNum());
         assertEquals("FIRST ROW == 0", 0, s.getFirstRowNum());
-
-        // assert((s.getLastRowNum() == 99));
     }
 
     /**
@@ -130,13 +118,10 @@ public final class TestWorkbook extends TestCase {
         HSSFRow          r    = null;
         HSSFCell         c    = null;
 
-        for (short rownum = ( short ) 0; rownum < 100; rownum++)
+        for (int rownum = 0; rownum < 100; rownum++) {
-        {
             r = s.createRow(rownum);
 
-            // r.setRowNum(( short ) rownum);
+            for (int cellnum = 0; cellnum < 50; cellnum += 2) {
-            for (short cellnum = ( short ) 0; cellnum < 50; cellnum += 2)
-            {
                 c = r.createCell(cellnum);
                 c.setCellValue(rownum * 10000 + cellnum
                                + ((( double ) rownum / 1000)
@@ -145,13 +130,11 @@ public final class TestWorkbook extends TestCase {
                 c.setCellValue(new HSSFRichTextString("TEST"));
             }
         }
-        for (short rownum = ( short ) 0; rownum < 25; rownum++)
+        for (int rownum = 0; rownum < 25; rownum++) {
-        {
             r = s.getRow(rownum);
             s.removeRow(r);
         }
-        for (short rownum = ( short ) 75; rownum < 100; rownum++)
+        for (int rownum = 75; rownum < 100; rownum++) {
-        {
             r = s.getRow(rownum);
             s.removeRow(r);
         }
@@ -428,12 +411,10 @@ public final class TestWorkbook extends TestCase {
         HSSFWorkbook     wb   = new HSSFWorkbook();
         HSSFSheet        s    = wb.createSheet();
 
-        for (short rownum = ( short ) 0; rownum < 100; rownum++)
+        for (int rownum = 0; rownum < 100; rownum++) {
-        {
             HSSFRow r = s.createRow(rownum);
 
-            for (short cellnum = ( short ) 0; cellnum < 50; cellnum += 2)
+            for (int cellnum = 0; cellnum < 50; cellnum += 2) {
-            {
                 HSSFCell c = r.createCell(cellnum);
                 c.setCellValue(rownum * 10000 + cellnum
                                + ((( double ) rownum / 1000)
@@ -465,13 +446,10 @@ public final class TestWorkbook extends TestCase {
     /**
      * Test the backup field gets set as expected.
      */
-
+    public void testBackupRecord() {
-    public void testBackupRecord()
+        HSSFWorkbook wb = new HSSFWorkbook();
-        throws Exception
+		wb.createSheet();
-    {
+		Workbook workbook = wb.getWorkbook();
-        HSSFWorkbook wb       = new HSSFWorkbook();
-        wb.createSheet();
-        Workbook     workbook = wb.getWorkbook();
         BackupRecord record   = workbook.getBackupRecord();
 
         assertEquals(0, record.getBackup());
@@ -479,7 +457,7 @@ public final class TestWorkbook extends TestCase {
         assertEquals(1, record.getBackup());
     }
 
-    private static final class RecordCounter implements ERFListener {
+    private static final class RecordCounter implements RecordVisitor {
         private int _count;
 
         public RecordCounter() {
@@ -488,9 +466,10 @@ public final class TestWorkbook extends TestCase {
         public int getCount() {
             return _count;
         }
-        public boolean processRecord(Record rec) {
+        public void visitRecord(Record r) {
-            _count++;
+            if (r instanceof LabelSSTRecord) {
-            return true;
+                _count++;
+            }
         }
     }
     
@@ -499,9 +478,7 @@ public final class TestWorkbook extends TestCase {
      *
      * We need to make sure only one LabelSSTRecord is produced.
      */
-    public void testRepeatingBug()
+    public void testRepeatingBug() {
-        throws Exception
-    {
         HSSFWorkbook workbook = new HSSFWorkbook();
         HSSFSheet    sheet    = workbook.createSheet("Design Variants");
         HSSFRow      row      = sheet.createRow(2);
@@ -510,12 +487,8 @@ public final class TestWorkbook extends TestCase {
         cell.setCellValue(new HSSFRichTextString("Class"));
         cell = row.createCell(2);
 
-        byte[] data = new byte[sheet.getSheet().getSize()];
-        sheet.getSheet().serialize(0, data);
         RecordCounter rc = new RecordCounter();
-        EventRecordFactory erf = new EventRecordFactory(rc, new short[] { LabelSSTRecord.sid, });
+        sheet.getSheet().visitContainedRecords(rc, 0);
-        erf.processRecords(new ByteArrayInputStream(data));
-        
         assertEquals(1, rc.getCount());
     }