nkuprins commented on issue #955:
URL: https://github.com/apache/fesod/issues/955#issuecomment-5032610441
> Based on my understanding, if explicit validation of unknown t attribute
is performed in the method `CellDataTypeEnum.buildFromCellType` and a clearer
exception is thrown (e.g., `throw new IllegalArgumentException("Invalid cell
data type: xxx")`), it would be more consistent with the fast-fail processing
logic.
>
> The reason for using `IllegalArgumentException` is that this is not a
logical exception in the parsing flow; treating it as an argument error is more
semantically appropriate here.
@bengbengbalabalabeng thanks for taking a look! One thing I noticed is that
throwing in `CellTagHandler.startElement` lets us point to the exact cell,
which can be useful for diagnostics:
```java
String cellType = attributes.getValue(ExcelXmlConstants.ATTRIBUTE_T);
CellDataTypeEnum type = CellDataTypeEnum.buildFromCellType(cellType);
if (type == null) {
throw new ExcelAnalysisException( // or IllegalArgumentException
"Invalid cell data type: '" + cellType + "' at row "
+ xlsxReadSheetHolder.getRowIndex() + ", column " +
xlsxReadSheetHolder.getColumnIndex());
}
xlsxReadSheetHolder.setTempCellData(new ReadCellData<>(type));
```
Would you say the clearer diagnostics (naming the exact row/column) outweigh
keeping the validation centralised in `buildFromCellType`?
- If **yes**: which exception would you prefer there -
`ExcelAnalysisException` or `IllegalArgumentException`?
- If **no**, then I think this is the version you have in mind:
```java
public static CellDataTypeEnum buildFromCellType(String cellType) {
if (StringUtils.isEmpty(cellType)) {
return EMPTY;
}
CellDataTypeEnum type = TYPE_ROUTING_MAP.get(cellType);
if (type == null) {
throw new IllegalArgumentException("Invalid cell data type: " +
cellType);
}
return type;
}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
