Some clean-up in BoundSheetRecord

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

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

@@ -326,15 +326,14 @@ public final class Workbook implements Model {
 
         int nBoundSheets = 1; // now just do 1
         for ( int k = 0; k < nBoundSheets; k++ ) {   
-            BoundSheetRecord bsr =
+            BoundSheetRecord bsr = retval.createBoundSheet(k);
-                    (BoundSheetRecord) retval.createBoundSheet( k );
 
-            records.add( bsr );
+			records.add(bsr);
-            retval.boundsheets.add( bsr );
+			retval.boundsheets.add(bsr);
-            retval.records.setBspos( records.size() - 1 );
+			retval.records.setBspos(records.size() - 1);
         }
-//        retval.records.supbookpos = retval.records.bspos + 1;
+        // retval.records.supbookpos = retval.records.bspos + 1;
-//        retval.records.namepos = retval.records.supbookpos + 2;
+        //        retval.records.namepos = retval.records.supbookpos + 2;
         records.add( retval.createCountry() );
         for ( int k = 0; k < nBoundSheets; k++ ) {   
             retval.getOrCreateLinkTable().checkExternSheet(k);
@@ -498,7 +497,6 @@ public final class Workbook implements Model {
         checkSheets(sheetnum);
         BoundSheetRecord sheet = (BoundSheetRecord)boundsheets.get( sheetnum );
         sheet.setSheetname(sheetname);
-        sheet.setSheetnameLength( (byte)sheetname.length() );
     }
 
     /**
@@ -519,22 +517,6 @@ public final class Workbook implements Model {
         return false;
     }
     
-    /**
-     * sets the name for a given sheet forcing the encoding. This is STILL A BAD IDEA.
-     * Poi now automatically detects unicode
-     *
-     *@deprecated 3-Jan-06 Simply use setSheetNam e(int sheetnum, String sheetname)
-     * @param sheetnum the sheet number (0 based)
-     * @param sheetname the name for the sheet
-     */    
-    public void setSheetName(int sheetnum, String sheetname, short encoding ) {
-        checkSheets(sheetnum);
-        BoundSheetRecord sheet = getBoundSheetRec(sheetnum);
-        sheet.setSheetname(sheetname);
-        sheet.setSheetnameLength( (byte)sheetname.length() );
-        sheet.setCompressedUnicodeFlag( (byte)encoding );
-    }
-    
     /**
      * sets the order of appearance for a given sheet.
      *
@@ -643,13 +625,12 @@ public final class Workbook implements Model {
      * if we're trying to address one more sheet than we have, go ahead and add it!  if we're
      * trying to address >1 more than we have throw an exception!
      */
-
     private void checkSheets(int sheetnum) {
         if ((boundsheets.size()) <= sheetnum) {   // if we're short one add another..
             if ((boundsheets.size() + 1) <= sheetnum) {
                 throw new RuntimeException("Sheet number out of bounds!");
             }
-            BoundSheetRecord bsr = (BoundSheetRecord ) createBoundSheet(sheetnum);
+            BoundSheetRecord bsr = createBoundSheet(sheetnum);
 
             records.add(records.getBspos()+1, bsr);
             records.setBspos( records.getBspos() + 1 );
@@ -1860,37 +1841,8 @@ public final class Workbook implements Model {
      * @see org.apache.poi.hssf.record.BoundSheetRecord
      * @see org.apache.poi.hssf.record.Record
      */
-
+    private static BoundSheetRecord createBoundSheet(int id) {
-    protected Record createBoundSheet(int id) {   // 1,2,3 sheets
+        return new BoundSheetRecord("Sheet" + (id+1));
-        BoundSheetRecord retval = new BoundSheetRecord();
-
-        switch (id) {
-
-            case 0 :
-                retval.setPositionOfBof(0x0);   // should be set later
-                retval.setOptionFlags(( short ) 0);
-                retval.setSheetnameLength(( byte ) 0x6);
-                retval.setCompressedUnicodeFlag(( byte ) 0);
-                retval.setSheetname("Sheet1");
-                break;
-
-            case 1 :
-                retval.setPositionOfBof(0x0);   // should be set later
-                retval.setOptionFlags(( short ) 0);
-                retval.setSheetnameLength(( byte ) 0x6);
-                retval.setCompressedUnicodeFlag(( byte ) 0);
-                retval.setSheetname("Sheet2");
-                break;
-
-            case 2 :
-                retval.setPositionOfBof(0x0);   // should be set later
-                retval.setOptionFlags(( short ) 0);
-                retval.setSheetnameLength(( byte ) 0x6);
-                retval.setCompressedUnicodeFlag(( byte ) 0);
-                retval.setSheetname("Sheet3");
-                break;
-        }
-        return retval;
     }
 
     /**

src/java/org/apache/poi/hssf/record/BoundSheetRecord.java

@@ -23,6 +23,7 @@ import java.util.List;
 
 import org.apache.poi.util.BitField;
 import org.apache.poi.util.BitFieldFactory;
+import org.apache.poi.util.HexDump;
 import org.apache.poi.util.LittleEndian;
 import org.apache.poi.util.StringUtil;
 
@@ -42,13 +43,13 @@ public final class BoundSheetRecord extends Record {
 	private static final BitField hiddenFlag = BitFieldFactory.getInstance(0x01);
 	private static final BitField veryHiddenFlag = BitFieldFactory.getInstance(0x02);
 	private int field_1_position_of_BOF;
-    private short field_2_option_flags;
+	private int field_2_option_flags;
-    private byte field_3_sheetname_length;
+	private int field_4_isMultibyteUnicode;
-    private byte field_4_compressed_unicode_flag;   // not documented
 	private String field_5_sheetname;
 
-    public BoundSheetRecord()
+	public BoundSheetRecord(String sheetname) {
-    {
+		field_2_option_flags = 0;
+		setSheetname(sheetname);
 	}
 
 	/**
@@ -56,115 +57,86 @@ public final class BoundSheetRecord extends Record {
 	 *
 	 * @param in the RecordInputstream to read the record from
 	 */
-
+	public BoundSheetRecord(RecordInputStream in) {
-    public BoundSheetRecord( RecordInputStream in )
+		super(in);
-    {
-        super( in );
 	}
 
-    protected void validateSid( short id )
+	protected void validateSid(short id) {
-    {
+		if (id != sid) {
-        if ( id != sid )
+			throw new RecordFormatException("NOT A Bound Sheet RECORD");
-        {
-            throw new RecordFormatException( "NOT A Bound Sheet RECORD" );
 		}
 	}
 
 	/**
-     *  UTF8:
+	 * UTF8: sid + len + bof + flags + len(str) + unicode + str 2 + 2 + 4 + 2 +
-     *	sid + len + bof + flags + len(str) + unicode +   str
+	 * 1 + 1 + len(str)
-     *	 2  +  2  +  4  +   2   +    1     +    1    + len(str)
 	 * 
-     * 	UNICODE:
+	 * UNICODE: sid + len + bof + flags + len(str) + unicode + str 2 + 2 + 4 + 2 +
-     *	sid + len + bof + flags + len(str) + unicode +   str
+	 * 1 + 1 + 2 * len(str)
-     *	 2  +  2  +  4  +   2   +    1     +    1    + 2 * len(str)
 	 * 
 	 */
+	protected void fillFields(RecordInputStream in) {
+		field_1_position_of_BOF = in.readInt();
+		field_2_option_flags = in.readUShort();
+		int field_3_sheetname_length = in.readUByte();
+		field_4_isMultibyteUnicode = in.readByte();
 
-    protected void fillFields( RecordInputStream in )
+		if (isMultibyte()) {
-    {
+			field_5_sheetname = in.readUnicodeLEString(field_3_sheetname_length);
-        field_1_position_of_BOF = in.readInt();	// bof
+		} else {
-        field_2_option_flags = in.readShort();	// flags
+			field_5_sheetname = in.readCompressedUnicode(field_3_sheetname_length);
-        field_3_sheetname_length = in.readByte();						// len(str)
-        field_4_compressed_unicode_flag = in.readByte();						// unicode
-
-        int nameLength = LittleEndian.ubyteToInt( field_3_sheetname_length );
-        if ( ( field_4_compressed_unicode_flag & 0x01 ) == 1 )
-        {
-            field_5_sheetname = in.readUnicodeLEString(nameLength);
-        }
-        else
-        {
-            field_5_sheetname = in.readCompressedUnicode(nameLength);
 		}
 	}
 
 	/**
-     * set the offset in bytes of the Beginning of File Marker within the HSSF Stream part of the POIFS file
+	 * set the offset in bytes of the Beginning of File Marker within the HSSF
+	 * Stream part of the POIFS file
 	 * 
-     * @param pos  offset in bytes
+	 * @param pos
+	 *			offset in bytes
 	 */
-
+	public void setPositionOfBof(int pos) {
-    public void setPositionOfBof( int pos )
-    {
 		field_1_position_of_BOF = pos;
 	}
 
-    /**
-     * set the option flags (unimportant for HSSF supported sheets)
-     *
-     * @param flags to set
-     */
-
-    public void setOptionFlags( short flags )
-    {
-        field_2_option_flags = flags;
-    }
-
-    /**
-     * Set the length of the sheetname in characters
-     *
-     * @param len  number of characters in the sheet name
-     * @see #setSheetname(String)
-     */
-
-    public void setSheetnameLength( byte len )
-    {
-        field_3_sheetname_length = len;
-    }
-
-    /**
-     * set whether or not to interperate the Sheetname as compressed unicode (8/16 bit)
-     * (This is undocumented but can be found as Q187919 on the Microsoft(tm) Support site)
-     * @param flag (0/1) 0- compressed, 1 - uncompressed (16-bit)
-     */
-
-    public void setCompressedUnicodeFlag( byte flag )
-    {
-        field_4_compressed_unicode_flag = flag;
-    }
-
 	/**
 	 * Set the sheetname for this sheet.  (this appears in the tabs at the bottom)
-     * @param sheetname the name of the sheet
+	 * @param sheetName the name of the sheet
 	 * @throws IllegalArgumentException if sheet name will cause excel to crash. 
 	 */
+	public void setSheetname(String sheetName) {
 		
-    public void setSheetname( String sheetname )
+		validateSheetName(sheetName);
-    {
+		field_5_sheetname = sheetName;
-        
+		field_4_isMultibyteUnicode = StringUtil.hasMultibyte(sheetName) ?  1 : 0;
-        if ((sheetname == null) || (sheetname.length()==0)
+	}
-                || (sheetname.length()>31)
+
-                || (sheetname.indexOf("/") > -1)
+	private static void validateSheetName(String sheetName) {
-                || (sheetname.indexOf("\\") > -1)
+		if (sheetName == null) {
-                || (sheetname.indexOf("?") > -1)
+			throw new IllegalArgumentException("sheetName must not be null");
-                || (sheetname.indexOf("*") > -1)
+		}
-                || (sheetname.indexOf("]") > -1)
+		int len = sheetName.length();
-                || (sheetname.indexOf("[") > -1) ){
+		if (len < 1 || len > 31) {
-                    throw new IllegalArgumentException("Sheet name cannot be blank, greater than 31 chars, or contain any of /\\*?[]");
+			throw new IllegalArgumentException("sheetName '" + sheetName 
+					+ "' is invalid - must be 1-30 characters long");
+		}
+		for (int i=0; i<len; i++) {
+			char ch = sheetName.charAt(i);
+			switch (ch) {
+				case '/':
+				case '\\':
+				case '?':
+				case '*':
+				case ']':
+				case '[':
+					break;
+				default:
+					// all other chars OK
+					continue;
+			}
+			throw new IllegalArgumentException("Invalid char (" + ch 
+					+ ") found at index (" + i + ") in sheet name '" + sheetName + "'");
 		}
-        field_5_sheetname = sheetname;
-        setCompressedUnicodeFlag(StringUtil.hasMultibyte(sheetname) ?  (byte)1 : (byte)0);        
  	}
 
 	/**
@@ -172,137 +144,62 @@ public final class BoundSheetRecord extends Record {
 	 *
 	 * @return offset in bytes
 	 */
-
+	public int getPositionOfBof() {
-    public int getPositionOfBof()
-    {
 		return field_1_position_of_BOF;
 	}
 
-    /**
+	private boolean isMultibyte() {
-     * get the option flags (unimportant for HSSF supported sheets)
+		return (field_4_isMultibyteUnicode & 0x01) != 0;
-     *
-     * @return flags to set
-     */
-
-    public short getOptionFlags()
-    {
-        return field_2_option_flags;
-    }
-
-    /**
-     * get the length of the sheetname in characters
-     *
-     * @return number of characters in the sheet name
-     * @see #getSheetname()
-     */
-
-    public byte getSheetnameLength()
-    {
-        return field_3_sheetname_length;
-    }
-
-    /**
-     * get the length of the raw sheetname in characters
-     * the length depends on the unicode flag
-     *
-     * @return number of characters in the raw sheet name
-     */
-
-    public byte getRawSheetnameLength()
-    {
-        return (byte) ( ( ( field_4_compressed_unicode_flag & 0x01 ) == 1 )
-                ? 2 * field_3_sheetname_length
-                : field_3_sheetname_length );
-    }
-
-    /**
-     * get whether or not to interperate the Sheetname as compressed unicode (8/16 bit)
-     * (This is undocumented but can be found as Q187919 on the Microsoft(tm) Support site)
-     * @return flag (0/1) 0- compressed, 1 - uncompressed (16-bit)
-     */
-
-    public byte getCompressedUnicodeFlag()
-    {
-        return field_4_compressed_unicode_flag;
 	}
 
 	/**
 	 * get the sheetname for this sheet.  (this appears in the tabs at the bottom)
 	 * @return sheetname the name of the sheet
 	 */
-
+	public String getSheetname() {
-    public String getSheetname()
-    {
 		return field_5_sheetname;
 	}
 
-    public String toString()
+	public String toString() {
-    {
 		StringBuffer buffer = new StringBuffer();
 
-        buffer.append( "[BOUNDSHEET]\n" );
+		buffer.append("[BOUNDSHEET]\n");
-        buffer.append( "    .bof             = " )
+		buffer.append("    .bof        = ").append(HexDump.intToHex(getPositionOfBof())).append("\n");
-                .append( Integer.toHexString( getPositionOfBof() ) ).append( "\n" );
+		buffer.append("    .options    = ").append(HexDump.shortToHex(field_2_option_flags)).append("\n");
-        buffer.append( "    .optionflags     = " )
+		buffer.append("    .unicodeflag= ").append(HexDump.byteToHex(field_4_isMultibyteUnicode)).append("\n");
-                .append( Integer.toHexString( getOptionFlags() ) ).append( "\n" );
+		buffer.append("    .sheetname  = ").append(field_5_sheetname).append("\n");
-        buffer.append( "    .sheetname length= " )
+		buffer.append("[/BOUNDSHEET]\n");
-                .append( Integer.toHexString( getSheetnameLength() ) ).append( "\n" );
-        buffer.append( "    .unicodeflag     = " )
-                .append( Integer.toHexString( getCompressedUnicodeFlag() ) )
-                .append( "\n" );
-        buffer.append( "    .sheetname       = " ).append( getSheetname() )
-                .append( "\n" );
-        buffer.append( "[/BOUNDSHEET]\n" );
 		return buffer.toString();
 	}
 	
-    public int serialize( int offset, byte[] data )
+	private int getDataSize() {
-    {
+		return 8 + field_5_sheetname.length() * (isMultibyte() ? 2 : 1);
-        LittleEndian.putShort( data, 0 + offset, sid );
-        LittleEndian.putShort( data, 2 + offset, (short) ( 8 + getRawSheetnameLength() ) );
-        LittleEndian.putInt( data, 4 + offset, getPositionOfBof() );
-        LittleEndian.putShort( data, 8 + offset, getOptionFlags() );
-        data[10 + offset] = (byte) ( getSheetnameLength() );
-        data[11 + offset] = getCompressedUnicodeFlag();
-
-        if ( ( field_4_compressed_unicode_flag & 0x01 ) == 1 )
-            StringUtil.putUnicodeLE( getSheetname(), data, 12 + offset );
-        else
-            StringUtil.putCompressedUnicode( getSheetname(), data, 12 + offset );
-
-
-        return getRecordSize();
-
-        /*
-		byte[] fake = new byte[] {	(byte)0x85, 0x00, 			// sid
-		    							0x1a, 0x00, 			// length
-		    							0x3C, 0x09, 0x00, 0x00, // bof
-		    							0x00, 0x00, 			// flags
-		    							0x09, 					// len( str )
-		    							0x01, 					// unicode
-		    							// <str>
-		    							0x21, 0x04, 0x42, 0x04, 0x40, 0x04, 0x30, 0x04, 0x3D,
-		    							0x04, 0x38, 0x04, 0x47, 0x04, 0x3A, 0x04, 0x30, 0x04
-		    							// </str>
-		    						};
-
-		    						sid + len + bof + flags + len(str) + unicode +   str
-		    						 2  +  2  +  4  +   2   +    1     +    1    + len(str)
-
-		System.arraycopy( fake, 0, data, offset, fake.length );
-
-		return fake.length;
-		*/
 	}
 
-    public int getRecordSize()
+	public int serialize(int offset, byte[] data) {
-    {
+		int dataSize = getDataSize();
-        // Includes sid length + size length
+		LittleEndian.putUShort(data, 0 + offset, sid);
-        return 12 + getRawSheetnameLength();
+		LittleEndian.putUShort(data, 2 + offset, dataSize);
+		LittleEndian.putInt(data, 4 + offset, getPositionOfBof());
+		LittleEndian.putUShort(data, 8 + offset, field_2_option_flags);
+
+		String name = field_5_sheetname;
+		LittleEndian.putByte(data, 10 + offset, name.length());
+		LittleEndian.putByte(data, 11 + offset, field_4_isMultibyteUnicode);
+
+		if (isMultibyte()) {
+			StringUtil.putUnicodeLE(name, data, 12 + offset);
+		} else {
+			StringUtil.putCompressedUnicode(name, data, 12 + offset);
+		}
+		return 4 + dataSize;
 	}
 
-    public short getSid()
+	public int getRecordSize() {
-    {
+		return 4 + getDataSize();
+	}
+
+	public short getSid() {
 		return sid;
 	}
 
@@ -317,7 +214,7 @@ public final class BoundSheetRecord extends Record {
 	 * Is the sheet hidden? Different from very hidden 
 	 */
 	public void setHidden(boolean hidden) {
-	    field_2_option_flags = hiddenFlag.setShortBoolean(field_2_option_flags, hidden);
+		field_2_option_flags = hiddenFlag.setBoolean(field_2_option_flags, hidden);
 	}
 
 	/**
@@ -331,33 +228,25 @@ public final class BoundSheetRecord extends Record {
 	 * Is the sheet very hidden? Different from (normal) hidden 
 	 */
 	public void setVeryHidden(boolean veryHidden) {
-	    field_2_option_flags = veryHiddenFlag.setShortBoolean(field_2_option_flags, veryHidden);
+		field_2_option_flags = veryHiddenFlag.setBoolean(field_2_option_flags, veryHidden);
 	}
 	
 	/**
-     * Takes a list of BoundSheetRecords, and returns the all
+	 * Converts a List of {@link BoundSheetRecord}s to an array and sorts by the position of their
-     *  ordered by the position of their BOFs.
+	 * BOFs.
 	 */
 	public static BoundSheetRecord[] orderByBofPosition(List boundSheetRecords) {
-    	BoundSheetRecord[] bsrs = (BoundSheetRecord[])boundSheetRecords.toArray(
+		BoundSheetRecord[] bsrs = new BoundSheetRecord[boundSheetRecords.size()];
-    			new BoundSheetRecord[boundSheetRecords.size()]);
+		boundSheetRecords.toArray(bsrs);
-    	
+		Arrays.sort(bsrs, BOFComparator);
-    	// Sort
-    	Arrays.sort(bsrs, new BOFComparator());
-    	
-    	// All done
 	 	return bsrs;
 	}
-    private static class BOFComparator implements Comparator {
+	private static final Comparator BOFComparator = new Comparator() {
 		public int compare(Object bsr1, Object bsr2) {
 			return compare((BoundSheetRecord)bsr1, (BoundSheetRecord)bsr2);
 		}
 		public int compare(BoundSheetRecord bsr1, BoundSheetRecord bsr2) {
-			if(bsr1.field_1_position_of_BOF < bsr2.field_1_position_of_BOF)
+			return bsr1.getPositionOfBof() - bsr2.getPositionOfBof();
-				return -1;
-			if(bsr1.field_1_position_of_BOF == bsr2.field_1_position_of_BOF)
-				return 0;
-			return 1;
-		}
 		}
+	};
 }

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

@@ -514,18 +514,6 @@ public class HSSFWorkbook extends POIDocument
         return (short) getFirstVisibleTab();
     }
 
-    /**
-     * @deprecated POI will now properly handle unicode strings without
-     * forceing an encoding
-     */
-    public final static byte ENCODING_COMPRESSED_UNICODE = 0;
-    /**
-     * @deprecated POI will now properly handle unicode strings without
-     * forceing an encoding
-     */
-    public final static byte ENCODING_UTF_16             = 1;
-
-
     /**
      * set the sheet name.
      * Will throw IllegalArgumentException if the name is greater than 31 chars
@@ -541,35 +529,6 @@ public class HSSFWorkbook extends POIDocument
         workbook.setSheetName(sheetIx, name);
     }
 
-
-    /**
-     * set the sheet name forcing the encoding. Forcing the encoding IS A BAD IDEA!!!
-     * @deprecated 3-Jan-2006 POI now automatically detects unicode and sets the encoding
-     * appropriately. Simply use setSheetName(int sheet, String encoding)
-     * @throws IllegalArgumentException if the name is greater than 31 chars
-     * or contains /\?*[]
-     * @param sheet number (0 based)
-     */
-    public void setSheetName(int sheetIx, String name, short encoding)
-    {
-        if (workbook.doesContainsSheetName( name, sheetIx )) {
-            throw new IllegalArgumentException( "The workbook already contains a sheet with this name" );
-        }
-        validateSheetIndex(sheetIx);
-
-        switch ( encoding ) {
-        case ENCODING_COMPRESSED_UNICODE:
-        case ENCODING_UTF_16:
-            break;
-
-        default:
-            // TODO java.io.UnsupportedEncodingException
-            throw new RuntimeException( "Unsupported encoding" );
-        }
-
-        workbook.setSheetName( sheetIx, name, encoding );
-    }
-
     /**
      * get the sheet name
      * @param sheetIx Number

src/testcases/org/apache/poi/hssf/record/TestBOFRecord.java

@@ -18,7 +18,6 @@
 package org.apache.poi.hssf.record;
 
 import java.io.InputStream;
-import java.util.ArrayList;
 
 import junit.framework.AssertionFailedError;
 import junit.framework.TestCase;
@@ -39,24 +38,4 @@ public final class TestBOFRecord extends TestCase {
             throw new AssertionFailedError("Identified bug 42794");
         }
     }
-    
-    public void testOrdering() throws Exception {
-    	BoundSheetRecord bs1 = new BoundSheetRecord();
-    	BoundSheetRecord bs2 = new BoundSheetRecord();
-    	BoundSheetRecord bs3 = new BoundSheetRecord();
-    	bs1.setPositionOfBof(11);
-    	bs2.setPositionOfBof(33);
-    	bs3.setPositionOfBof(22);
-    	
-    	ArrayList l = new ArrayList();
-    	l.add(bs1);
-    	l.add(bs2);
-    	l.add(bs3);
-    	
-    	BoundSheetRecord[] r = BoundSheetRecord.orderByBofPosition(l);
-    	assertEquals(3, r.length);
-    	assertEquals(bs1, r[0]);
-    	assertEquals(bs3, r[1]);
-    	assertEquals(bs2, r[2]);
-    }
 }

src/testcases/org/apache/poi/hssf/record/TestBoundSheetRecord.java

@@ -1,4 +1,3 @@
-
 /* ====================================================================
    Licensed to the Apache Software Foundation (ASF) under one or more
    contributor license agreements.  See the NOTICE file distributed with
@@ -16,9 +15,11 @@
    limitations under the License.
 ==================================================================== */
 
-
 package org.apache.poi.hssf.record;
 
+import java.util.ArrayList;
+
+import junit.framework.AssertionFailedError;
 import junit.framework.TestCase;
 
 /**
@@ -28,53 +29,77 @@ import junit.framework.TestCase;
  *
  * @author Glen Stampoultzis (glens at apache.org)
  */
-public class TestBoundSheetRecord
+public final class TestBoundSheetRecord extends TestCase {
-        extends TestCase
+
-{
+
-    public TestBoundSheetRecord( String s )
+    public void testRecordLength() {
-    {
+        BoundSheetRecord record = new BoundSheetRecord("Sheet1");
-        super( s );
+        assertEquals(18, record.getRecordSize());
     }
 
-    public void testRecordLength()
+    public void testWideRecordLength() {
-            throws Exception
+        BoundSheetRecord record = new BoundSheetRecord("Sheet\u20ac");
-    {
+        assertEquals(24, record.getRecordSize());
-        BoundSheetRecord record = new BoundSheetRecord();
-        record.setCompressedUnicodeFlag((byte)0x00);
-        record.setSheetname("Sheet1");
-        record.setSheetnameLength((byte)6);
-
-        assertEquals(" 2  +  2  +  4  +   2   +    1     +    1    + len(str)", 18, record.getRecordSize());
-    }
-
-    public void testWideRecordLength()
-            throws Exception
-    {
-        BoundSheetRecord record = new BoundSheetRecord();        
-        record.setSheetname("Sheet\u20ac");
-        record.setSheetnameLength((byte)6);
-
-        assertEquals(" 2  +  2  +  4  +   2   +    1     +    1    + len(str) * 2", 24, record.getRecordSize());
     }
 
     public void testName() {
-        BoundSheetRecord record = new BoundSheetRecord();
+        BoundSheetRecord record = new BoundSheetRecord("1234567890223456789032345678904");
-        record.setSheetname("1234567890223456789032345678904");
+
-        assertTrue("Success", true);
         try {
             record.setSheetname("12345678902234567890323456789042");
-            assertTrue("Should have thrown IllegalArgumentException, but didnt", false);
+            throw new AssertionFailedError("Should have thrown IllegalArgumentException, but didnt");
         } catch (IllegalArgumentException e) {
-            assertTrue("succefully threw exception",true);
+            // expected
         }
 
         try {
             record.setSheetname("s//*s");
-            assertTrue("Should have thrown IllegalArgumentException, but didnt", false);
+            throw new AssertionFailedError("Should have thrown IllegalArgumentException, but didnt");
         } catch (IllegalArgumentException e) {
-            assertTrue("succefully threw exception",true);
+            // expected
         }
+    }
+    
+    public void testDeserializeUnicode() {
+       
+		byte[] data = {
+//			(byte)0x85, 0x00,       // sid
+//			0x1a, 0x00, 			// length
+			0x3C, 0x09, 0x00, 0x00, // bof
+			0x00, 0x00, 			// flags
+			0x09, 					// len( str )
+			0x01, 					// unicode
+			// <str>
+			0x21, 0x04, 0x42, 0x04, 0x40, 0x04, 
+			0x30, 0x04, 0x3D, 0x04, 0x38, 0x04,
+			0x47, 0x04, 0x3A, 0x04, 0x30, 0x04
+			// </str>
+		};
+	
+		RecordInputStream in = new TestcaseRecordInputStream(BoundSheetRecord.sid, data);
+		BoundSheetRecord bsr = new BoundSheetRecord(in);
+		// sheet name is unicode Russian for 'minor page'
+		assertEquals("\u0421\u0442\u0440\u0430\u043D\u0438\u0447\u043A\u0430", bsr.getSheetname());
 		
 	}
 
+    public void testOrdering() {
+    	BoundSheetRecord bs1 = new BoundSheetRecord("SheetB");
+    	BoundSheetRecord bs2 = new BoundSheetRecord("SheetC");
+    	BoundSheetRecord bs3 = new BoundSheetRecord("SheetA");
+    	bs1.setPositionOfBof(11);
+    	bs2.setPositionOfBof(33);
+    	bs3.setPositionOfBof(22);
+
+    	ArrayList l = new ArrayList();
+    	l.add(bs1);
+    	l.add(bs2);
+    	l.add(bs3);
+
+    	BoundSheetRecord[] r = BoundSheetRecord.orderByBofPosition(l);
+    	assertEquals(3, r.length);
+    	assertEquals(bs1, r[0]);
+    	assertEquals(bs3, r[1]);
+    	assertEquals(bs2, r[2]);
+    }
 }

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

@@ -17,13 +17,20 @@
 
 package org.apache.poi.hssf.usermodel;
 
-import junit.framework.TestCase;
-import junit.framework.AssertionFailedError;
-
-import java.util.List;
 import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
 
-import org.apache.poi.hssf.record.*;
+import junit.framework.AssertionFailedError;
+import junit.framework.TestCase;
+
+import org.apache.poi.hssf.record.BOFRecord;
+import org.apache.poi.hssf.record.BoundSheetRecord;
+import org.apache.poi.hssf.record.EOFRecord;
+import org.apache.poi.hssf.record.InterfaceHdrRecord;
+import org.apache.poi.hssf.record.NameRecord;
+import org.apache.poi.hssf.record.Record;
+import org.apache.poi.hssf.usermodel.SanityChecker.CheckRecord;
 
 /**
  * A Test case for a test utility class.<br/>
@@ -33,23 +40,26 @@ import org.apache.poi.hssf.record.*;
  */
 public final class TestSanityChecker extends TestCase {
 
+	private static BoundSheetRecord createBoundSheetRec() {
+		return new BoundSheetRecord("Sheet1");
+	}
 	public void testCheckRecordOrder() {
 		final SanityChecker c = new SanityChecker();
 		List records = new ArrayList();
 		records.add(new BOFRecord());
 		records.add(new InterfaceHdrRecord());
-        records.add(new BoundSheetRecord());
+		records.add(createBoundSheetRec());
 		records.add(EOFRecord.instance);
-        final SanityChecker.CheckRecord[] check = {
+		CheckRecord[] check = {
-            new SanityChecker.CheckRecord(BOFRecord.class, '1'),
+				new CheckRecord(BOFRecord.class, '1'),
-            new SanityChecker.CheckRecord(InterfaceHdrRecord.class, '0'),
+				new CheckRecord(InterfaceHdrRecord.class, '0'),
-            new SanityChecker.CheckRecord(BoundSheetRecord.class, 'M'),
+				new CheckRecord(BoundSheetRecord.class, 'M'),
-            new SanityChecker.CheckRecord(NameRecord.class, '*'),
+				new CheckRecord(NameRecord.class, '*'),
-            new SanityChecker.CheckRecord(EOFRecord.class, '1'),
+				new CheckRecord(EOFRecord.class, '1'),
 		};
 		// check pass
 		c.checkRecordOrder(records, check);
-        records.add(2, new BoundSheetRecord());
+		records.add(2, createBoundSheetRec());
 		c.checkRecordOrder(records, check);
 		records.remove(1);	  // optional record missing
 		c.checkRecordOrder(records, check);
@@ -58,112 +68,68 @@ public final class TestSanityChecker extends TestCase {
 		c.checkRecordOrder(records, check);
 
 		// check fail
-        expectFail( new Runnable() {
+		confirmBadRecordOrder(check, new Record[] {
-            public void run()
+				new BOFRecord(),
-            {
+				createBoundSheetRec(),
-                // check optional in wrong spot
+				new InterfaceHdrRecord(),
-                List records = new ArrayList();
+				EOFRecord.instance,
-                records.add(new BOFRecord());
-                records.add(new BoundSheetRecord());
-                records.add(new InterfaceHdrRecord());
-                records.add(EOFRecord.instance);
-                c.checkRecordOrder(records, check);
-            }
 		});
 
-        expectFail( new Runnable() {
+		confirmBadRecordOrder(check, new Record[] {
-            public void run()
+				new BOFRecord(),
-            {
+				new InterfaceHdrRecord(),
-                // check optional one off occurs more than once
+				createBoundSheetRec(),
-                List records = new ArrayList();
+				new InterfaceHdrRecord(),
-                records.add(new BOFRecord());
+				EOFRecord.instance,
-                records.add(new InterfaceHdrRecord());
-                records.add(new BoundSheetRecord());
-                records.add(new InterfaceHdrRecord());
-                records.add(EOFRecord.instance);
-                c.checkRecordOrder(records, check);
-            }
 		});
 
-        expectFail( new Runnable() {
+		confirmBadRecordOrder(check, new Record[] {
-            public void run()
+				new BOFRecord(),
-            {
+				createBoundSheetRec(),
-                // check many scattered
+				new NameRecord(),
-                List records = new ArrayList();
+				EOFRecord.instance,
-                records.add(new BOFRecord());
+				new NameRecord(),
-                records.add(new BoundSheetRecord());
-                records.add(new NameRecord());
-                records.add(EOFRecord.instance);
-                records.add(new NameRecord());
-                c.checkRecordOrder(records, check);
-            }
 		});
 
-        expectFail( new Runnable() {
+		confirmBadRecordOrder(check, new Record[] {
-            public void run()
+				new InterfaceHdrRecord(),
-            {
+				createBoundSheetRec(),
-                // check missing manditory
+				EOFRecord.instance,
-                List records = new ArrayList();
-                records.add(new InterfaceHdrRecord());
-                records.add(new BoundSheetRecord());
-                records.add(EOFRecord.instance);
-                c.checkRecordOrder(records, check);
-            }
 		});
 
-        expectFail( new Runnable() {
+		confirmBadRecordOrder(check, new Record[] {
-            public void run()
+				new BOFRecord(),
-            {
+				new InterfaceHdrRecord(),
-                // check missing 1..many
+				EOFRecord.instance,
-                List records = new ArrayList();
-                records.add(new BOFRecord());
-                records.add(new InterfaceHdrRecord());
-                records.add(EOFRecord.instance);
-                c.checkRecordOrder(records, check);
-            }
 		});
 
-        expectFail( new Runnable() {
+		confirmBadRecordOrder(check, new Record[] {
-            public void run()
+				new InterfaceHdrRecord(),
-            {
+				createBoundSheetRec(),
-                // check wrong order
+				new BOFRecord(),
-                List records = new ArrayList();
+				EOFRecord.instance,
-                records.add(new InterfaceHdrRecord());
-                records.add(new BoundSheetRecord());
-                records.add(new BOFRecord());
-                records.add(EOFRecord.instance);
-                c.checkRecordOrder(records, check);
-            }
 		});
 
-        expectFail( new Runnable() {
+		confirmBadRecordOrder(check, new Record[] {
-            public void run()
+				new BOFRecord(),
-            {
+				createBoundSheetRec(),
-                // check optional record in wrong order
+				new InterfaceHdrRecord(),
-                List records = new ArrayList();
+				EOFRecord.instance,
-                records.add(new BOFRecord());
+		});
-                records.add(new BoundSheetRecord());
+	}
-                records.add(new InterfaceHdrRecord());
+	private static void confirmBadRecordOrder(final SanityChecker.CheckRecord[] check, Record[] recs) {
-                records.add(EOFRecord.instance);
+		final SanityChecker c = new SanityChecker();
+		final List records = Arrays.asList(recs);
+		try {
+			new Runnable() {
+				public void run() {
 					c.checkRecordOrder(records, check);
 				}
-        });
+			}.run();
-
+		} catch (AssertionFailedError pass) {
+			// expected during normal test
+			return;
 		}
-
+		throw new AssertionFailedError("Did not get failure exception as expected");
-    private void expectFail( Runnable runnable )
-    {
-        boolean fail = false;
-        try
-        {
-            runnable.run();
-            fail = true;
 	}
-        catch (AssertionFailedError pass)
-        {
-        }
-        assertTrue(!fail);
-    }
-
 }
-