Bug 49873

Summary:	XSSFFormulaEvaluator.evaluate() can not handle blank cells
Product:	POI	Reporter:	alex <i8c.alex>
Component:	XSSF	Assignee:	POI Developers List <dev>
Status:	RESOLVED FIXED
Severity:	normal
Priority:	P2
Version:	3.6-dev
Target Milestone:	---
Hardware:	PC
OS:	Windows XP
Attachments:	Creates a static method to create a cellvalue with type CELL_TYPE_BLANK Updates XSSFFormulaEvaluator to allow for evaluation of blank cells

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