https://issues.apache.org/bugzilla/show_bug.cgi?id=48292
--- Comment #1 from Josh Micich <[email protected]> 2009-11-30 16:25:38 UTC --- This was originally attachment ID 24632 that was lost as part of the issues.apache.org data loss on 2009-11-26/27. Note that this attachment ID has since been re-used. The original attachment comment was: 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: [email protected] For additional commands, e-mail: [email protected]
