Bug 64964 - HSSFCell.convertCellValueToBoolean shall throw more specific exception
Summary: HSSFCell.convertCellValueToBoolean shall throw more specific exception
Status: RESOLVED FIXED
Alias: None
Product: POI
Classification: Unclassified
Component: HSSF (show other bugs)
Version: unspecified
Hardware: PC All
: P2 normal (vote)
Target Milestone: ---
Assignee: POI Developers List
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2020-12-08 06:52 UTC by zhonghao
Modified: 2020-12-08 17:13 UTC (History)
0 users



Attachments

Note You need to log in before you can comment on or make changes to this bug.
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