Bug 47809 - [PATCH] Improved work with UDFs.
Summary: [PATCH] Improved work with UDFs.
Status: RESOLVED FIXED
Alias: None
Product: POI
Classification: Unclassified
Component: POI Overall (show other bugs)
Version: unspecified
Hardware: PC Windows XP
: P2 normal (vote)
Target Milestone: ---
Assignee: POI Developers List
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-09-09 04:20 UTC by Petr.Udalau
Modified: 2009-09-16 17:46 UTC (History)
0 users



Attachments
Patch with implementation. (26.78 KB, text/plain)
2009-09-09 04:20 UTC, Petr.Udalau
Details
zip with patch and TestData (10.54 KB, patch)
2009-09-15 05:06 UTC, Petr.Udalau
Details | Diff

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