Bug 63118 - proposal: make Cell.setCellType(CellType.FORMULA) illegal
Summary: proposal: make Cell.setCellType(CellType.FORMULA) illegal
Status: RESOLVED FIXED
Alias: None
Product: POI
Classification: Unclassified
Component: SS Common (show other bugs)
Version: 4.0.x-dev
Hardware: PC All
: P2 enhancement (vote)
Target Milestone: ---
Assignee: POI Developers List
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2019-01-26 15:59 UTC by gallon.fizik@gmail.com
Modified: 2020-07-28 09:38 UTC (History)
0 users



Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description gallon.fizik@gmail.com 2019-01-26 15:59:36 UTC
Motivation: as well as any setCellType call doesn't make much sense and implies implicit conversions/default value setting, calling setCellType(CellType.FORMULA) makes even less sense because formula has no default value and cannot be null. Setting formula to "0" is a workaround and, again, is an implicit (i.e. counter-intuitive) side effect.

In all the test suite, I didn't find any reasonable usage of this call. There were a bunch of calls in test SetUp methods (obviously, redundant) and a call in XSSFCell ctor. *Perhaps* it wins a tiny bit in terms of performance, although I very much doubt it.

I have the change ready and tested but am open to obections/discussion. The change is documented in Cell interface.
Comment 1 Andreas Beeker 2019-01-26 16:13:50 UTC
I'm +1 for making it illegal

- throw an exception with a hint (to using setCellFormula instead?) now
- mark setCellType deprecated for removal in POI 5

please also check the examples/docs for references to it
Comment 2 gallon.fizik@gmail.com 2019-01-26 19:56:35 UTC
Implemented in r1852244 - r1852247.

Changes:
* setCellType is now deprecated (@Removal = 5.0)
* setCellType(FORMULA) on a non-formula cell is illegal and throws an IAE with a message suggesting to use setCellFormula directly.
* setCellType(FORMULA) on a formula cell does nothing
* added Cell.setBlank() to be the only use case that will survive the removal of setCellType. Current implementation delegates to setCellType(BLANK), but at least that's not a part of the public API.
* purged most calls to setCellType within the project. setCellType(BLANK) were replaced with setBlank, calls with other arguments were removed wherever possible (i.e. where it didn't break tests). Mostly these calls were in the test suite. The only (if I didn't miss something) calls were left in tests to verify value conversions. This logic/tests will pass away when setCellType is removed.
Comment 3 Alex 2020-07-28 09:38:29 UTC
The suggesion, to use setCellValue(...) instead would imply that the value is known, it is not. Where as using `setCellType`, the value has not to be known. `getStringCellValue()` is not working properly, if the CellType was not changed to String beforehand. How can this be accomplished now, with such a deprecation, that offers no substitute for the lost functionality?