Bug 46301

Summary: [PATCH] Added code to parse some of the PivotTable-related records
Product: POI Reporter: Patrick Cheng <patrickyccheng>
Component: HSSFAssignee: POI Developers List <dev>
Status: RESOLVED FIXED    
Severity: normal    
Priority: P2    
Version: unspecified   
Target Milestone: ---   
Hardware: Macintosh   
OS: Mac OS X 10.4   
Attachments: diff
new files

Description Patrick Cheng 2008-11-26 09:03:08 UTC 
created StreamID, ViewSource, PageItem, ViewDefinition, ViewFields, DataItem, ExtendedPivotTableViewFields.

Added reference to RecordFactory and BiffViewer, removed references in UnknownRecord.

Had to add new functions in StringUtils to read/write unicode string with length not located immediately before the string.
Comment 1 Patrick Cheng 2008-11-26 09:11:20 UTC 
Created attachment 22952 [details]
diff
Comment 2 Patrick Cheng 2008-11-26 09:11:34 UTC 
Created attachment 22953 [details]
new files
Comment 3 Patrick Cheng 2008-11-26 09:12:46 UTC 
forgot to prefix [PATCH]
Comment 4 Josh Micich 2008-11-26 15:16:36 UTC 
Applied in svn r721007

Thanks for the recent patches (bug 46189 too).  So far these changes don't add any functionality (POI can't do anything with charts or pivot tables yet).  What level of support are you aiming for (e.g. 'read-only access', 'modify existing', or 'create from scratch')?

BTW - some comments on the code you've been submitting:
(1) - add the standard apache header in all new files
(2) - add (at least brief) class comments on new classes
(3) - use java int for 16 bit values instead of short and byte.  In BIFF records, most 16 bit ints are really unsigned, and using java short can cause big trouble.  Another annoying problem is that java demands casts on integer constants assigned to short variables even when they are clearly within the range of a short.  In most places (fields, local variables, parameters) there is no JVM memory savings. Primitive bytes, shorts, chars, and ints all take 4 bytes (an exception is in primitive arrays, which seem to be stored compactly in most JVMs).
(4) -  Please write junits for the new classes - at the very least for the complex parts.  As they stand, these recent patches actually make POI less reliable because data which was previously read and written by UnknownRecord (using one very simple implementation) is now handled by many different classes, none of which have tests cases.  Classes like PageItemRecord are perhaps so simple that they don't need unit tests, but ViewDefinitionRecord definitely should.  As a matter of fact, I found a bug in ViewFieldsRecord.serialize(LittleEndianOutput), concerning writing the string length twice.