This patch modifies HSSFWorkbook, FormulaParser and SheetReferences to allow sheet references to be captured from HSSFWorkbook (new method getSheetReferences, which mirrors Workbook.getSheetReferences()) and passed into the FormulaParser (new methods getSheetReferences() and setSheetReferences()). I purposefully didn't add a new constructor for FormulaParser(String, SheetReferences) because of the common practice up to now of just calling 'new FormulaParser(formula, null)' (null for the workbook). That call produces an ambiguous call error. Instead I added the setSheetReferences, so the typical way to call it now would be something like: POIFSFileSystem fs = new POIFSFileSystem(new FileInputStream(file)); HSSFWorkbook wb = new HSSFWorkbook(fs); HSSFSheet sheet = wb.getSheetAt(0); HSSFRow row = sheet.getRow(0); SheetReferneces refs = wb.getSheetReferences(); FormulaParser fp = new FormulaParser(formula, null); fp.setSheetReferences(refs); fp.parse(); Also, modifications to FormulaParser were required to check whether to get the sheet numbers from the workbook, if valid, or the sheet references, if supplied. This also required a new method getSheetIndex() in SheetReferences (the counterpart to getSheetName()). A unit test was added to TestSheetReferences to test the reverse lookups. Unit tests weren't added yet to test the advanced formula parsing, as I'm not that up on JUnit yet, but if anyone has an pointers, I'm all ears.
Created attachment 7539 [details] Patch for 3DRef resolving in a formua (HSSFWorkbook, SheetReferences and FormulaParser patched).
com'on JUnit isn't hard to use. go LOOK at the unit tests. You're telling me you need pointers? The only pointer I have is write the unit test before the code. I have to admit I don't favor this patch for the same reason I don't want to open up the model. This smells like a hack and I think we can do something better. I'm retargeting it to 3.0, but I favor a different approach possibly just refactoring where needed. I do appreciate your contribution though.
Here's a pointer: avoid assert and assertTrue. If the test involves checking two values for equality, use assertEquals. For example, assertEquals("ref A", 0, refs.getSheetIndex("A")); Put the expected result on the left and the actual result on the right. This will give you much more descriptive error messages when something goes wrong.
Ok, ok.. I'll poke around with JUnit.. :) As for the assertTrue, I was just following the previous examlpe in TestSheetReferehces. Regardless of how it's refactored, somehow, the FormulaParser needs information out of the workbook, or access to the workbook to resolve 3DRefs. Getting a copy of the SheetReferences seemed like less of a hack than exposing the internal workbook to the outside world. At least it can't be used in reverse to mess up the workbook. Admittedly, it's still a hack, though.
Right, there is no need for dirty hacks in 3.0. Play with the scaffolding a bit. Try to fix the frame a bit.
This patch will now certainly not applied since FormulaParser etc has been refactored by the macro function patch of paul krause. The functionality of this patch may however still have to be added. To be investigated!
Ok, so formula parser has moved on, but without any testcases, I have no clue what problem this patch is supposed to solve! So while this looks useful at first glance, I am finding it diffcult to figure this out! Can someone atleast give me a worksheet, or formulas, that this solves?
The problem was that parsing a 3DRef requires the sheet references in the formula parser, but there was no way to actually get the sheet references from inside the parser. The two solutions I toyed around with was this one (getting the references from teh worksheet, then passing them to the parser) and making the lower workbook available to HSSFWorkbook by making the Workbook.getWorkbook() method public. The second method was poo-pooed. I can create some test cases in about a week. I'm about to go AFK to a conference and won't be able to get back to this in a while.
Created attachment 15050 [details] Test class that exhibits the problem This java code exibits the problem with parsing 3d refs.
Created attachment 15051 [details] spreadsheet that shows the problem this is a 3 sheet workbook that has a formula on sheet 3 that's a the sum of sheet1!A1 and sheet2!A2.
I attached two additional files that show the problem this patch is trying to overcome. If you run the TestClass on test.xls, it'll blow up in FormulaParser, line 276. This is because FormulaParser expects to be passed an object of type Workbook so that it can resolve 3DRefs. Line 276 is calling the checkExternSheet() method of the Workbook class. Since you can't get a Workbook object from the HSSFWorkbook, it blows up. The two solutions are (1) make getWorkbook in Workbook public so you can get it and pass it to the FormulaParser or (2) modify Formula parser to accept SheetReferences that are obtained from the Workbook. The patch attached to this bug implements option 2 above since exposing the Workbook directly was generally regarded as a "bad idea".
I've been trying to bring the patch foward and ran into an interesting chicken/egg problem. In every Ptg class, the toForumlaString method takes a second parameter of type Workbook. If there's no way to get a Workbook from an HSSFWorkbook, none of those parameters can ever be filled with anything other than null. I was reworking the patch slightly from the mind-set that you can never obtain an object of type Workbook to pass anywhere, so I was removing references of Workbook from the FormulaParser. Then I found all the Ptg's. From a quick analysis, it looks like it can safely be removed from all of them (it's unused in 90% of them, only Area3DPtg and Ref3DPtg actually use it and it's just to get the sheet references). The only one I'm concerned about is the NamePtg. I'm kind of hesitant to produce a patch that touches that many files, but it really does seem to me that the whole involvement of Workbook in the formula related classes is completely useless and can never be utilized. Period. Should I continue?
Try the latest POI-3.5-beta4, it supports 3DRefs. Yegor