Bug 46301 - [PATCH] Added code to parse some of the PivotTable-related records
Summary: [PATCH] Added code to parse some of the PivotTable-related records
Alias: None
Product: POI
Classification: Unclassified
Component: HSSF (show other bugs)
Version: unspecified
Hardware: Macintosh Mac OS X 10.4
: P2 normal (vote)
Target Milestone: ---
Assignee: POI Developers List
Depends on:
Reported: 2008-11-26 09:03 UTC by Patrick Cheng
Modified: 2008-11-26 15:16 UTC (History)
0 users

diff (6.80 KB, patch)
2008-11-26 09:11 UTC, Patrick Cheng
Details | Diff
new files (3.22 KB, patch)
2008-11-26 09:11 UTC, Patrick Cheng
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
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]
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.