fastexcel
fastexcel copied to clipboard
dhatim
Suggestion: Return null instead of throwing OutOfBoundsException when reading Cells from Row
First of all, I really appreciate the work that went into this!
I'm parsing a xlsx with 36 columns or so, and the last few columns represent optional data. As such, them being empty/null is a valid state.
However, when I read a cell that is null, and all following columns of that row are also null, "OutOfBoundsException" is thrown.
As per the Excel specs, the maximum column number is 16,384.
If feasible, I propose throwing "OutOfBoundsException", when the column index greater than 16,384 is requested, and otherwise return null on absent values.
Cheers
Thank you! Changing this behavior is possible, but I fear this might introduce regressions for existing users. We could do this is a new minor version and document it thoroughly. What do you think?
From semver docs: "Major version zero (0.y.z) is for initial development. Anything MAY change at any time. The public API SHOULD NOT be considered stable."
IMHO projects relying on this library at its 0.y.z stage should cover their usage of it with tests, and expect breaking changes at any time.
That being said, if you agree with the behavior as I have described it, then yes, I think a minor version as you have suggested would be an appropriate way to reflect the change.
BTW, why return null, not Optional, if the lib relies on java 8? Yeah, optional is a bit ugly in java, but it's better than pure null, or use some nullable annotations on methods
I'm consuming this library from a Kotlin codebase, so personally I don't mind the null as the language allows me to deal with it quite nicely.
I don't think it makes much of a difference, different folks probably have different preferences/opinions here.
Kotlin works better with java when at least annotations are used instead of simple nulls https://kotlinlang.org/docs/reference/java-interop.html#nullability-annotations
Or you could implement an overload which would return null for blank cells, like Apache POI does with
row.getCell(index, Row.RETURN_BLANK_AS_NULL);
Or you could implement an overload which would return null for blank cells, like Apache POI does with
row.getCell(index, Row.RETURN_BLANK_AS_NULL);
We could indeed use this or something like row.getCellOrNull() to keep the API compatible.
