|Summary:||[PATCH] Improved work with UDFs.|
|Component:||POI Overall||Assignee:||POI Developers List <dev>|
Patch with implementation.
zip with patch and TestData
Description Petr.Udalau 2009-09-09 04:20:11 UTC
Created attachment 24235 [details] Patch with implementation. Systematized work of ToolPacks(Created ToolPack interface and class MainToolPacksHandler). Improved resolving of UDFs(in class UserDefinedFunction). Workbook now contains Map of UDFs by name(added methods to register and find).
Comment 1 Yegor Kozlov 2009-09-13 09:25:32 UTC
Petr, The patch looks OK, but I'm very reluctant to apply it without a single unit test. Can you upload some? Unit tests are needed not only for quality assurance, but they also serve as code examples helping users to understand how the code works and how to use it. Regards, Yegor
Comment 2 Petr.Udalau 2009-09-15 05:06:58 UTC
Created attachment 24268 [details] zip with patch and TestData There is a patch with test. Test: internal VB function and function in ToolPack. Also there is one problem with Names of Workbook: if I set value for cell inside the POI cell.setCellFormula("ISODD(1)")(function "ISODD" defined in AnalysisToolPak) error will occur: org.apache.poi.ss.formula.FormulaParser$FormulaParseException: Name 'ISODD' is completely unknown in the current workbook at org.apache.poi.ss.formula.FormulaParser.function(FormulaParser.java:932) at org.apache.poi.ss.formula.FormulaParser.parseNonRange(FormulaParser.java:570) at org.apache.poi.ss.formula.FormulaParser.parseRangeable(FormulaParser.java:442) at org.apache.poi.ss.formula.FormulaParser.parseRangeExpression(FormulaParser.java:285) at org.apache.poi.ss.formula.FormulaParser.parseSimpleFactor(FormulaParser.java:1131) at org.apache.poi.ss.formula.FormulaParser.percentFactor(FormulaParser.java:1091) at org.apache.poi.ss.formula.FormulaParser.powerFactor(FormulaParser.java:1078) at org.apache.poi.ss.formula.FormulaParser.Term(FormulaParser.java:1400) at org.apache.poi.ss.formula.FormulaParser.additiveExpression(FormulaParser.java:1500) at org.apache.poi.ss.formula.FormulaParser.concatExpression(FormulaParser.java:1484) at org.apache.poi.ss.formula.FormulaParser.comparisonExpression(FormulaParser.java:1441) at org.apache.poi.ss.formula.FormulaParser.unionExpression(FormulaParser.java:1421) at org.apache.poi.ss.formula.FormulaParser.parse(FormulaParser.java:1542) at org.apache.poi.ss.formula.FormulaParser.parse(FormulaParser.java:193) at org.apache.poi.hssf.model.HSSFFormulaParser.parse(HSSFFormulaParser.java:69) at org.apache.poi.hssf.usermodel.HSSFCell.setCellFormula(HSSFCell.java:598). Work with Names must be fixed.
Comment 3 Yegor Kozlov 2009-09-16 12:22:59 UTC
Applied in r815928 FormulaParseException is another issue, I'm sure we will fix it in nearest future. Thanks, Yegor
Comment 4 Josh Micich 2009-09-16 17:46:47 UTC
Major modifications made in svn r816016 There were a few problems with the code as originally submitted: (1) Naming - The name "ToolPack" is not general enough ("AnalysisToolPak" is just one "Add-in" library). The sub-package has been changed to "udf". The classes have been changed to "*UDFFinder". "UDFFinder" refers to any collection of UDFs: external ("Add-ins" libraries) or internal (VB modules). (2) Mutability - Implementors of UDFFinder should be immutable (there is no need to change UDFs once evaluation has begun). (3) Registration - since we are only dealing with evaluation of UDFs, this a concern of the formula evaluator (not the xSSFWorkbook). The patch code seemed to suggest that it was also maintaining the defined name records within the workbook, but that code looked too simple to be complete. I am pretty sure that additional updates needed elsewhere in the LinkTable, like the SUPBOOK records. We will probably need some method that does this (creating DEFINEDNAME records etc for UDFs used in a workbook) to address the parser error you describe above. In any case, the workbook does not need to know about the the UDF evaluation implementation - e.g. the user may be writing formulas but not evaluating them. Please take a look at the changes and make sure that the functionality you desire is still available.