Bug 21673

Summary: [PATCH] SheetReferences in FormulaParser.toString is not created correctly
Product: POI Reporter: Eric Ladner <eladner>
Component: HSSFAssignee: POI Developers List <dev>
Status: RESOLVED WONTFIX    
Severity: normal    
Priority: P3    
Version: 2.0-pre3   
Target Milestone: ---   
Hardware: All   
OS: All   
Attachments: refs assignment fix for FormulaParser.toString()

Description Eric Ladner 2003-07-17 01:12:39 UTC
In the toString method of FormulaParser, the SheetReferences variable refs is
not correctly assigned a value, making parsed formulas drop 3DRefs when printing
formula output.  As far as I can tell, this only affects raw formula output and
not formula parsing in general.
Comment 1 Eric Ladner 2003-07-17 01:13:45 UTC
Created attachment 7343 [details]
refs assignment fix for FormulaParser.toString()
Comment 2 Eric Ladner 2003-07-17 01:15:03 UTC
'refs' isn't assigned correctly in FormulaParser.toString().  Patch below (and
also attached as an individual patch file)

--- jakarta-poi-original/src/java/org/apache/poi/hssf/model/FormulaParser.java 
2003-07-16 20:00:46.000000000 -0500
+++ jakarta-poi/src/java/org/apache/poi/hssf/model/FormulaParser.java  
2003-07-16 20:05:59.000000000 -0500
@@ -891,7 +891,9 @@
      */
     public String toString() {
         SheetReferences refs = null;
-        if (book!=null)  book.getSheetReferences();
+        if (book!=null)  {
+            refs = book.getSheetReferences();
+        }
         StringBuffer buf = new StringBuffer();
            for (int i=0;i<tokens.size();i++) {
             buf.append( ( (Ptg)tokens.get(i)).toFormulaString(refs));
Comment 3 Paul Krause 2003-07-17 15:11:16 UTC
Patch 7337 for bug 21444 also fixes this, but in a different way.  If that 
patch is applied, then this one is not needed.
Comment 4 Andy Oliver 2003-07-24 17:14:34 UTC
should be evaluated.  A test case would be REALLY nice. :-(
Comment 5 Eric Ladner 2003-07-26 19:49:52 UTC
I'm fairly NOOB when it comes to JUnit, but I'll take a look.  The other problem
is that this patch is really aimed at resolving 3DRefs, which when parsing
formulas, is rather useless without the HSSFWorkbook.getWorkbook() being public.
 As far as I can tell, the only way to get the formula parser to be aware of the
sheet names to resolve references is to pass in the workbook to the parser.  In
short, it'd be hard to write a unit test aginst the stock tree without that
patch or some other method to get the workbook into the parser.

In fact, I can't see any way for a class outside HSSFWorkbook to be able to pass
the workbook into the FormulaParser constructor without the calling class
extending HSSFWorkbook.

I haven't investigated using the low level Workbook for this yet, however.
Comment 6 Andy Oliver 2003-07-26 20:23:39 UTC
The unit tests are all in the SAME package as the classes they test.  They are just "seperate" 
meaning they aren't packaged in the final jar.  Thus they have full access to "workbook"..  I'm never 
ever going to let up on the making the model public.  In fact...I'm going to become an even worse 
encapsulation fanatic and be even more ZEALOUS about seperating the two.  The model is not for 
the user level.  
Comment 7 Avik Sengupta 2003-07-31 18:39:14 UTC
I think we should go with the approach in bug 21444 .. Thats solves this and
many others.