Bug 49873 - XSSFFormulaEvaluator.evaluate() can not handle blank cells
Summary: XSSFFormulaEvaluator.evaluate() can not handle blank cells
Status: RESOLVED FIXED
Alias: None
Product: POI
Classification: Unclassified
Component: XSSF (show other bugs)
Version: 3.6-dev
Hardware: PC Windows XP
: P2 normal (vote)
Target Milestone: ---
Assignee: POI Developers List
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-09-03 02:27 UTC by alex
Modified: 2010-09-06 06:44 UTC (History)
0 users



Attachments
Creates a static method to create a cellvalue with type CELL_TYPE_BLANK (455 bytes, patch)
2010-09-03 02:31 UTC, alex
Details | Diff
Updates XSSFFormulaEvaluator to allow for evaluation of blank cells (558 bytes, patch)
2010-09-03 02:31 UTC, alex
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description alex 2010-09-03 02:27:48 UTC
If you try to evaluate() a blank cell, you will get an IllegalStateException() because cellType "3" is not known (Cell.CELL_TYPE_BLANK = 3).

This can be solved in the parent code by checking if the cell is blank before evaluating, but I think it would be easier to allow you to evaluate a blank cell at which point you simply receive a CellValue with type blank.
Comment 1 alex 2010-09-03 02:31:01 UTC
Created attachment 25977 [details]
Creates a static method to create a cellvalue with type CELL_TYPE_BLANK

Please note that the patches have not yet been tested.
Comment 2 alex 2010-09-03 02:31:50 UTC
Created attachment 25978 [details]
Updates XSSFFormulaEvaluator to allow for evaluation of blank cells
Comment 3 Yegor Kozlov 2010-09-04 11:07:45 UTC
Fixed in r992620.

The correct behavior is to return null - FormulaEvaluator should behave the same whether the cell is null or blank. This is how HSSF's evaluator is implemented and I don't see why XSSF should behave differently.

Regards,
Yegor
Comment 4 alex 2010-09-06 01:44:05 UTC
I do not know if it is allowed to comment on resolved issues (if not, please excuse my ignorance), but I would like to point out that while I understand that you want both implementations to work the same, I think it would be easier to return a blank cell in the following situation:

CellValue cellValue = evaluator.evaluate(cell);
switch(cellValue.getCellType()) {
...
}

This will throw a nullpointer exception if evaluator returns "null" for blank cells. It is of course entirely feasible to check for "== null" before doing the switch, but isn't that partly the point of having a "CELL_TYPE_BLANK"?
Comment 5 Yegor Kozlov 2010-09-06 06:44:29 UTC
(In reply to comment #4)
> I do not know if it is allowed to comment on resolved issues (if not, please
> excuse my ignorance), but I would like to point out that while I understand
> that you want both implementations to work the same, I think it would be easier
> to return a blank cell in the following situation:
> 
> CellValue cellValue = evaluator.evaluate(cell);
> switch(cellValue.getCellType()) {
> ...
> }
> 
> This will throw a nullpointer exception if evaluator returns "null" for blank
> cells. It is of course entirely feasible to check for "== null" before doing
> the switch, but isn't that partly the point of having a "CELL_TYPE_BLANK"?


what you suggest makes sense,  but current code exists for a long time and used in many production systems. Changing the returned value from null to CellValue.BLANK  may cause incompatibility issues. This is the main reason why I had to reject it.

Yegor