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 13:32:17 UTC

[Bug 58403] New: Require SpreadsheetVersion on CellRangeAddress.isFullColumnRange and isFullRowRange

https://bz.apache.org/bugzilla/show_bug.cgi?id=58403

            Bug ID: 58403
           Summary: Require SpreadsheetVersion on
                    CellRangeAddress.isFullColumnRange and isFullRowRange
           Product: POI
           Version: unspecified
          Hardware: PC
                OS: Linux
            Status: NEW
          Severity: normal
          Priority: P2
         Component: SS Common
          Assignee: dev@poi.apache.org
          Reporter: javenoneal@gmail.com

Created attachment 33100
  --> https://bz.apache.org/bugzilla/attachment.cgi?id=33100&action=edit
CellRangeAddressBase.java patch plus unit tests

This patch resolves a couple TODO's in org.apache.poi.ss.util.CellRangeAddress.

I see three options here:
1) Require the caller to supply the spreadsheet version as an argument to
isFullColumnRange and isFullRowRange, deprecate the zero-argument version of
these methods.
2) Add a SpreadsheetVersion instance variable to this class, set by the
constructor (either a final instance variable or with a setter).
3) Let the implementing class (org.apache.poi.hssf.util.CellRangeAddress)
override the zero-argument isFullColumnRange to specify the SpreadsheetVersion.

The first method seems best, as it has the least impact on existing projects,
reduces amount of RAM consumed by this object, and resembles other classes that
have been modified with SpreadsheetVersion. The downside is the caller (rather
than creator for Option 2) needs to know the SpreadsheetVersion.

There seems to be a lot of overlap between AreaReference and CellRangeAddress
org.apache.poi.ss.util.AreaReference[1]. In r1685101, AreaReference took the
direction of #2 above, adding a SpreadsheetVersion instance variable and
deprecating constructors that don't specify the spreadsheet version.
AreaReference#isWholeColumnReference() seems to behave similarly to
CellRangeAddress#isFullColumnRange. Oddly, AreaReference doesn't include a
isWholeRowReference method. CellRangeAddressBase stores 4 ints while
AreaReference stores 2 CellReferences. Key difference: References include
absolute/relative fields while Addresses do not.

Seems like some refactoring is needed here, perhaps merging these classes or
having these classes subclassed from a common class. Anyone have any strong
opinions on what direction should be taken?

[1]
https://svn.apache.org/viewvc/poi/trunk/src/java/org/apache/poi/ss/util/AreaReference.java?revision=1685101&view=markup#l194

-- 
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 58403] Require SpreadsheetVersion on CellRangeAddress.isFullColumnRange and isFullRowRange

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=58403

Javen ONeal <ja...@gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #33237|0                           |1
        is obsolete|                            |

--- Comment #2 from Javen ONeal <ja...@gmail.com> ---
Created attachment 33238
  --> https://bz.apache.org/bugzilla/attachment.cgi?id=33238&action=edit
CellRangeAddressBase.java patch plus unit tests

Fix whitespace and hashCode implementation.

-- 
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 58403] Require SpreadsheetVersion on CellRangeAddress.isFullColumnRange and isFullRowRange

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=58403

Javen ONeal <ja...@gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #33100|0                           |1
        is obsolete|                            |

--- Comment #1 from Javen ONeal <ja...@gmail.com> ---
Created attachment 33237
  --> https://bz.apache.org/bugzilla/attachment.cgi?id=33237&action=edit
CellRangeAddressBase.java patch plus unit tests

Rebasing patch due to conflicts with bug 58441: define equals method for
CellRangeAddressBase, needed for a different bug.

Also fixed broken hashCode test.

-- 
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 58403] Require SpreadsheetVersion on CellRangeAddress.isFullColumnRange and isFullRowRange

Posted by bu...@apache.org.
https://bz.apache.org/bugzilla/show_bug.cgi?id=58403

Dominik Stadler <do...@gmx.at> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Severity|normal                      |enhancement

-- 
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