https://issues.apache.org/bugzilla/show_bug.cgi?id=46301


Josh Micich <[EMAIL PROTECTED]> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|                            |FIXED




--- Comment #4 from Josh Micich <[EMAIL PROTECTED]>  2008-11-26 15:16:36 PST ---
Applied in svn r721007 ( https://svn.apache.org/viewcvs.cgi?view=rev&rev=721007 
)

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.


-- 
Configure bugmail: https://issues.apache.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.

---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]

Reply via email to