Skip to content

Prevent NPE on XWPFTableCell.getWidth() when tcW is not set.#1185

Draft
jaragunde wants to merge 1 commit into
apache:trunkfrom
jaragunde:npe-xwpftablecell-width
Draft

Prevent NPE on XWPFTableCell.getWidth() when tcW is not set.#1185
jaragunde wants to merge 1 commit into
apache:trunkfrom
jaragunde:npe-xwpftablecell-width

Conversation

@jaragunde

Copy link
Copy Markdown
Member

Returns -1 mirroring the behavior of XWPFTable.getWidth(), although getWidthDecimal() returns 0 in that case, for both XWPFTable and XWPFTableCell. The standard implies, in a roundabout way, that the value should default to 0 but -1 can be, and may have already been, used as a marker of "presence" by clients of the POI API.

Returns -1 mirroring the behavior of XWPFTable.getWidth(), although
getWidthDecimal() returns 0 in that case, for both XWPFTable and
XWPFTableCell. The standard implies, in a roundabout way, that the
value should default to 0 but -1 can be, and may have already been,
used as a marker of "presence" by clients of the POI API.
@jaragunde

Copy link
Copy Markdown
Member Author

@pjfanning I would like to know your opinion about returning -1 or 0 for XWPFTableCell.getWidth() when there's no explicit width set in the document. XWPFTable.getWidth() returns -1, but ``XWPFTable|XWPFTableCell.getWidthDecimal()` both return 0. I'm for using -1, as I explain in the message.

Heads-up: I will still work a bit on this topic; while I was testing this I realized it's possible to crash with NPE if the tcW element of the cell is set but empty.

@pjfanning

Copy link
Copy Markdown
Member

@jaragunde I wouldn't favour changing the existing method to return -1 when it never did before. Breaks backward compatibility. Introduce a new method or overload this method but include a new param, maybe a boolean, to control whether -1 or an exception is thrown in the not set case.
To be honest, it's not nice but compatibility guarantees matter. My preferred solution would be to add an isSet method that returns a boolean if tcW is set and to update the getWidth javadoc to mention that that is how you get work out the is not set case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants