Bug 64964

Summary: HSSFCell.convertCellValueToBoolean shall throw more specific exception
Product: POI Reporter: zhonghao
Component: HSSFAssignee: POI Developers List <dev>
Status: RESOLVED FIXED    
Severity: normal    
Priority: P2    
Version: unspecified   
Target Milestone: ---   
Hardware: PC   
OS: All   

Description zhonghao 2020-12-08 06:52:17 UTC
Given an unknown cell type, this method throws RuntimeException:

   private boolean convertCellValueToBoolean() {

        switch (_cellType) {
            case BOOLEAN:
                return (( BoolErrRecord ) _record).getBooleanValue();
            case STRING:
                int sstIndex = ((LabelSSTRecord)_record).getSSTIndex();
                String text = _book.getWorkbook().getSSTString(sstIndex).getString();
                return Boolean.parseBoolean(text);
            case NUMERIC:
                return ((NumberRecord)_record).getValue() != 0;

            case FORMULA:
                // use cached formula result if it's the right type:
                FormulaRecord fr = ((FormulaRecordAggregate)_record).getFormulaRecord();
                checkFormulaCachedValueType(CellType.BOOLEAN, fr);
                return fr.getCachedBooleanValue();
            // Other cases convert to false
            // These choices are not well justified.
            case ERROR:
            case BLANK:
                return false;
        }
        throw new RuntimeException("Unexpected cell type (" + _cellType + ")");
    }

It shall throw more specific exceptions. For example, ExcelComparator.compareDataInCell throws IllegalStateException:

 private void compareDataInCell(Locator loc1, Locator loc2) {
        if (isCellTypeMatches(loc1, loc2)) {
            final CellType loc1cellType = loc1.cell.getCellType();
            switch(loc1cellType) {
                case BLANK:
                case STRING:
                case ERROR:
                    isCellContentMatches(loc1,loc2);
                    break;
                case BOOLEAN:
                    isCellContentMatchesForBoolean(loc1,loc2);
                    break;
                case FORMULA:
                    isCellContentMatchesForFormula(loc1,loc2);
                    break;
                case NUMERIC:
                    if (DateUtil.isCellDateFormatted(loc1.cell)) {
                        isCellContentMatchesForDate(loc1,loc2);
                    } else {
                        isCellContentMatchesForNumeric(loc1,loc2);
                    }
                    break;
                default:
                    throw new IllegalStateException("Unexpected cell type: " + loc1cellType);
            }
        }
       ...
    }

The latter is more reasonable, and easy to be caught.
Comment 1 PJ Fanning 2020-12-08 17:12:18 UTC
Merged r1884211
Comment 2 PJ Fanning 2020-12-08 17:13:02 UTC
And r1884210