arjansh commented on a change in pull request #234: MM-82 Detect Column Types
URL: https://github.com/apache/metamodel/pull/234#discussion_r355413920
##########
File path: excel/src/main/java/org/apache/metamodel/excel/ExcelUtils.java
##########
@@ -264,12 +338,13 @@ private static String getFormulaCellValue(Workbook wb,
Cell cell) {
return getCellValue(wb, evaluatedCell);
} catch (RuntimeException e) {
- logger.warn("Exception occurred while evaluating formula at
position ({},{}): {}",
- new Object[] { cell.getRowIndex(), cell.getColumnIndex(),
e.getMessage() });
+ logger
+ .warn("Exception occurred while evaluating formula at
position ({},{}): {}", cell.getRowIndex(),
+ cell.getColumnIndex(), e.getMessage());
// Some exceptions we simply log - result will be then be the
// actual formula
if (e instanceof FormulaParseException) {
- logger.error("Parse exception occurred while evaluating cell
formula: " + cell, e);
+ logger.warn("Parse exception occurred while evaluating cell
formula: " + cell, e);
Review comment:
I would not touch this code, unless you really improve it, which would be to
get rid of the if (e instanceof ...Exception) construction and just use normal
catches instead, a bit like this:
```
} catch (FormulaParseException e) {
logger.warn("Parse exception occurred while evaluating cell
formula: " + cell, e);
} catch (IllegalArgumentException e) {
logger.warn("Illegal formula argument occurred while evaluating
cell formula: " + cell, e);
} catch (RuntimeException e) {
logger
.warn("Exception occurred while evaluating formula at
position ({},{}): {}", cell.getRowIndex(),
cell.getColumnIndex(), e.getMessage());
}
```
But in general, given the content of the different logged messages, I would
just catch the runtime exception, and get rid of the complete if then else
construction with all the different log message, which, to me, don't add any
real value as they won't make it any more clear where what went wrong in what
manner.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
