Bug 44050

Summary: [PATCH] Trim function in formula cell
Product: POI Reporter: Manda Wilson <wilson>
Component: HSSFAssignee: POI Developers List <dev>
Status: RESOLVED FIXED    
Severity: enhancement    
Priority: P5    
Version: unspecified   
Target Milestone: ---   
Hardware: All   
OS: All   
Attachments: Generated using svn diff > FormulaTrimFunction.patch
Contains the files I modified to implement and test the TRIM function.

Description Manda Wilson 2007-12-10 18:14:24 UTC
I have implemented the TRIM function for formula cells.  In Excel TRIM is
defined as "Removes all spaces from text except for single spaces between words.".  

I have also filled in the TRIM "FORMULA" and "EXPECTED VALUE" rows of the file
src/scratchpad/testcases/org/apache/poi/hssf/data/FormulaEvalTestData.xls with
15 test cases.  These test cases include passing TRIM a reference to a cell with
an empty string, passing TRIM numbers, passing TRIM an area, and others.  All
tests pass.

I would like to implement more formula functions, so any feedback on this one
would be greatly appreciated.  I plan to try MID next.
Comment 1 Manda Wilson 2007-12-10 18:16:24 UTC
Created attachment 21254 [details]
Generated using svn diff > FormulaTrimFunction.patch

This is a patch file, generated by running:

svn diff > FormulaTrimFunction.patch

After I finished implementing the TRIM function.
Comment 2 Manda Wilson 2007-12-10 18:22:19 UTC
Created attachment 21255 [details]
Contains the files I modified to implement and test the TRIM function.

This tar file contains two files that I modified to implement and test the TRIM
formula function.  

The first file is:

src/scratchpad/src/org/apache/poi/hssf/record/formula/functions/Trim.java

This is where the TRIM function is implemented.

The second file is:

src/scratchpad/testcases/org/apache/poi/hssf/data/FormulaEvalTestData.xls

I modified two rows in this file (rows 1392 and 1393), the "FORMULA" and
"EXPECTED VALUE" rows for the trim function.  This row contains the test cases
for trim.  These tests run successfully and are run automatically when the ant
task "test" is executed.
Comment 3 Nick Burch 2007-12-11 05:05:26 UTC
Thanks, committed

One thing to note is that not all of the contents of FormulaEvalTestData.xls
isn't checked automatically. Once your function is working, you need to list the
row number in
src/scratchpad/testcases/org/apache/poi/hssf/record/formula/eval/TestEverything.java
, and then it'll get tested for you (I've added the Trim row number)
Comment 4 Manda Wilson 2007-12-11 10:21:28 UTC
Thanks Nick.  Actually, I was wondering about the testing.  Trim was being tested in:

src/scratchpad/testcases/org/apache/poi/hssf/record/formula/functions/TestEverything.java

Which appears to set up a new GenericFormulaTestCase for every function listed in 
FormulaEvalTestData.xls between rows 80 and 1481.  Row 80 marks the beginning of the Excel 
functions section.

I know it was testing trim successfully because while developing I kept running:

ant -Dtestcase=org.apache.poi.hssf.record.formula.functions.TestEverything single-scratchpad-test

Which was failing on trim for a while.

I was wondering why some functions are listed explicitly in the file you mentioned, 
src/scratchpad/testcases/org/apache/poi/hssf/record/formula/eval/TestEverything.java, when the 
same test is run in 
src/scratchpad/testcases/org/apache/poi/hssf/record/formula/functions/TestEverything.java.  So far 
the only overlaps are concatenate, int, trim and upper.  I suspect that TestEverything.java in the eval 
package was meant only to test things like "=B7<B8" and not in fact the excel functions.

(In reply to comment #3)
> Thanks, committed
> 
> One thing to note is that not all of the contents of FormulaEvalTestData.xls
> isn't checked automatically. Once your function is working, you need to list the
> row number in
> src/scratchpad/testcases/org/apache/poi/hssf/record/formula/eval/TestEverything.java
> , and then it'll get tested for you (I've added the Trim row number)


Comment 5 Nick Burch 2007-12-12 02:31:04 UTC
Ah, you're right.

I've added some comments on this to the top of the two test files, which will
hopefully clear up this confusion for the next person!