DO NOT REPLY [Bug 48292] [PATCH] Support of array formulas

[bu...@apache.org]

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

--- Comment #1 from Josh Micich <jo...@gildedtree.com> 2009-11-27 14:35:52 UTC ---
Created an attachment (id=24632)
 --> (https://issues.apache.org/bugzilla/attachment.cgi?id=24632)
Some improvements to the original patch

Thanks for your work on array formulas.  This will be an important addition,
since this area is barely covered by POI at the moment (as I'm sure you're
aware).

There are a few issues with the rest of the patch, so I have re-worked what I
can and attached a new draft of the patch (based on the latest svn r885007). 
Some of my fixes might be wrong, so feel free to change anything to improve the
patch.

A very small amount of your first patch was applied in svn r885006.

The biggest outstanding concern is the lack of test cases.  Please write
something that calls the new top-level APIs.  Try to make the tests execute the
newly changed/added code in all the lower classes. 

In XSSFCell you wrote "if (isPartOfArrayFormulaGroup() && f == null)"
suggesting that it is possible for "isPartOfArrayFormulaGroup() to return true
with f!=null.  It seems more logical that this condition would represent a
corrupted spreadsheet.

According to OOO docs the formula parsing rules are slightly different for
array formulas.  You should use FormulaType.ARRAY, and we'll probably need to
update FormulaParser accordingly.

ConstantValueParser - it seems that there are now two alternate ways to encode
array element values of type text (with String and UnicodeString).  It would be
much better if such an ambiguity was not introduced into the code.  In
addition, UnicodeStrings can contain a list of FormattingRuns which is
something that is not applicable to array formulas (as far as I understand). 
According to the current junit tests, this code is not executed anyway.

FormulaRecordAggregate - field _sharedValueManager can never be null so there
is no need to check for that.  In general don't write null checks for things
that are asserted or documented as being never null - it raises doubts as to
whether the check might really be required.

In CellRangeAddress.valueOf() you added code which presumably allows one to
write valueOf("A") instead of valueOf("A:A") but it is not clear how this is
useful or relevant.

Try to keep classes encapsulated - for example the ArrayRecords list should not
spill out of SharedValueManager, and SharedValueManager should not spill out of
FormulaRecordAggregate.

Please don't use your own internal user-id for @author tags.  Full name in
plain text is best.

I also changed some method names and javadoc on the public API for clarity.

-- 
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: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org