You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@poi.apache.org by bu...@apache.org on 2015/09/13 12:31:57 UTC
[Bug 58402] New: AreaReference's SpreadsheetVersion instance
variable should be final
https://bz.apache.org/bugzilla/show_bug.cgi?id=58402
Bug ID: 58402
Summary: AreaReference's SpreadsheetVersion instance variable
should be final
Product: POI
Version: 3.13-dev
Hardware: PC
OS: Linux
Status: NEW
Severity: enhancement
Priority: P2
Component: SS Common
Assignee: dev@poi.apache.org
Reporter: javenoneal@gmail.com
CC: dtn-asfbugs@corefiling.co.uk
Depends on: 56328
Created attachment 33099
--> https://bz.apache.org/bugzilla/attachment.cgi?id=33099&action=edit
AreaReference.java patch
>From Bug 56328 (implemented in poi-3.13-beta1) r1685101
AreaReference's SpreadsheetVersion instance variable, _version is private and
is unable to be modified outside of the constructors (no setters provided).
This field should probably be final like the rest of the instance variables in
this class.
AreaReference(CellReference, CellReference) currently assumes
SpreadsheetVersion is EXCEL97. I deprecated this constructor in favor of
AreaReference(CellReference, CellReference, SpreadsheetVersion). I wasn't sure
what the preferred argument order was here, since the other constructor is
AreaReference(String, SpreadsheetVersion), but the static methods list
SpreadsheetVersion as the first argument. Your call.
I added a few other lines of defensive code to prevent NPEs.
One thing lead to another and I wrote unit tests for nearly all of
AreaReference. Note: many of these tests currently fail. In general, this class
should probably either delegate to a common cell reference string parser or
remove functions that operate on strings. It seems like allowing Strings that
do not match "[A-Z]+[0-9]+(:[A-Z]+[0-9]+)?" or perhaps even
"[A-Z]+[0-9]+:[A-Z]+[0-9]+" should raise an IllegalArgumentException--it
shouldn't be AreaReference's responsibility to strip off sheet information and
know sheet (and external workbook) naming rules.
Note: this patch requires further work before it can be committed.
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org
[Bug 58402] Rework AreaReference implementation details and tests
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=58402
Dominik Stadler <do...@gmx.at> changed:
What |Removed |Added
----------------------------------------------------------------------------
Summary|AreaReference's |Rework AreaReference
|SpreadsheetVersion instance |implementation details and
|variable should be final |tests
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org
[Bug 58402] AreaReference's SpreadsheetVersion instance variable
should be final
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=58402
--- Comment #1 from Javen ONeal <ja...@gmail.com> ---
Applied some of the internal changes from attachment 33099 in r1710163.
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org
[Bug 58402] AreaReference's SpreadsheetVersion instance variable
should be final
Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=58402
Javen ONeal <ja...@gmail.com> changed:
What |Removed |Added
----------------------------------------------------------------------------
Attachment #33099|0 |1
is obsolete| |
--- Comment #2 from Javen ONeal <ja...@gmail.com> ---
Created attachment 33198
--> https://bz.apache.org/bugzilla/attachment.cgi?id=33198&action=edit
AreaReference.java.patch
Rebased to r1710170.
In this patch:
* deprecate AreaReference(CellReference, CellReference), in favor of
AreaReference(CellReference, CellReference, SpreadsheetVersion)
* use static constants where ever possible
* pre-allocate a fixed-length list for getAllReferencedCells
* add many much-needed unit tests
Remaining work: fix either the unit tests or the code
For example, AreaReference("A1:B65536", EXCEL97).isWholeColumn() is false
because it doesn't include dollar signs. AreaReference("A:B").isWholeColumn()
is false because it doesn't include numbers. This doesn't seem like the correct
behavior.
--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org