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 2012/07/03 10:30:44 UTC
[Bug 53500] New: [Patch] Getter for repeating rows and columns
https://issues.apache.org/bugzilla/show_bug.cgi?id=53500
Priority: P2
Bug ID: 53500
Assignee: dev@poi.apache.org
Summary: [Patch] Getter for repeating rows and columns
Severity: normal
Classification: Unclassified
OS: All
Reporter: j.herrmann@web.de
Hardware: All
Status: NEW
Version: unspecified
Component: POI Overall
Product: POI
Created attachment 29023
--> https://issues.apache.org/bugzilla/attachment.cgi?id=29023&action=edit
zip containing patch, test case, and test data
[Patch] Getter for repeating rows and columns
Hello,
class Workbook provides a method
setRepeatingRowsAndColumns(int,int,int,int,int), but no corresponding getter.
The provided patch adds the following methods to class Sheet:
- CellRangeAddress getRepeatingRows()
- CellRangeAddress getRepeatingColumns()
A few notes & thoughts:
1) the method setRepeatingRowsAndColumns(...) is defined in Workbook, although
repeating rows/columns are actually Sheet properties. The rationale for the
assignment is given in the Javadoc:
"... This is function is included in the workbook because it
creates/modifies name records which are stored at the workbook level..."
For this are purely technical reasons, I would rather declare this method
@deprecated and move it to class Sheet, in order to improve class coherence.
2) It would be preferrable to split setRepeatingRowsAndColumns(...) into two
methods setRepeatingRows(String rowRangeRef) and setRepeatingColumns(String
columnRangeRef), as this maps more directly to the user interface fields, the
getters, and it avoids slightly puzzling -1 parameters when the user only wants
to define either repeating rows or columnns. E.g.:
setRepeatingRows("2:3") or setRepeatingColumns("A:A")
A null parameter would indicate that repeating rows/columns should be removed.
3) Regarding the returned class, CellRangeAddress: it appears that both
AreaReference and CellRangeAddress have some limitations when it comes to
handling Excel Version 97 versus 2007: both classes are a bit shaky when they
are to decide if a cell range spans full rows/columns, as the different Excel
versions support different maximum rows and columns.
CellReference, which is used by both CellRangeAddress and AreaReference, has an
inconspicuous comment which says :
"...// TODO - "-1" is a special value being temporarily used for whole row and
whole column area references. .."
This is a quite informal specification for what I think is an important
convention, as it offers a way to declare a whole row or column range in a
spreadsheet version-independent way. Thus, I decided to stress this feature
when using CellRangeAddress. However, the -1 convention seems to be implemented
only in a sporadic selection of methods. E.g.
CellRangeAddress.getNumberOfCells() returns wondrous results when operating
with -1.
3.1) A side note on CellRangeAddress and AreaReference: it seems that the
reference classes do not exploit the full power that a rich class hierarchy
could offer:
E.g. in various contexts, a CellRangeAdress (or AreaReference) parameter is not
exactly what is permitted, but only an apporximation.
An elaboorate reference class hierarchy could declare valid values more
precisely. E.g.:
CellSetRef <---+---- CellRangeRef <---------+----- CellRef
| \ \
| \ \
+----- RowSetRef <----]\[-- RowRangeRef <--- RowRef
| \
| \
+---ColumnSetRef <--- ColumnRangeRef <----- ColumnRef
Regards,
Joachim
--
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 53500] [Patch] Getter for repeating rows and columns
Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=53500
Yegor Kozlov <ye...@dinom.ru> changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|REOPENED |RESOLVED
Resolution|--- |FIXED
--- Comment #4 from Yegor Kozlov <ye...@dinom.ru> ---
Thanks for the follow-up,
patch applied in r1369290
Regards,
Yegor
--
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 53500] [Patch] Getter for repeating rows and columns
Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=53500
Yegor Kozlov <ye...@dinom.ru> changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|NEW |RESOLVED
Resolution|--- |FIXED
--- Comment #1 from Yegor Kozlov <ye...@dinom.ru> ---
Thanks for the excellent patch, applied in r1364061.
>
> 1) the method setRepeatingRowsAndColumns(...) is defined in Workbook,
> although repeating rows/columns are actually Sheet properties. The rationale
> for the assignment is given in the Javadoc:
> "... This is function is included in the workbook because it
> creates/modifies name records which are stored at the workbook level..."
> For this are purely technical reasons, I would rather declare this method
> @deprecated and move it to class Sheet, in order to improve class coherence.
>
I'm OK to deprecate Workbook.setRepeatingRowsAndColumns. If we make this change
we will need to change poi-examples to use the new methods.
> 2) It would be preferrable to split setRepeatingRowsAndColumns(...) into two
> methods setRepeatingRows(String rowRangeRef) and setRepeatingColumns(String
> columnRangeRef), as this maps more directly to the user interface fields,
> the getters, and it avoids slightly puzzling -1 parameters when the user
> only wants to define either repeating rows or columnns. E.g.:
> setRepeatingRows("2:3") or setRepeatingColumns("A:A")
> A null parameter would indicate that repeating rows/columns should be
> removed.
>
Sounds good. I'd prefer setRepeatingRows(CellRangeAddress rowRangeRef) , in
this case the type of the argument is the same in both getter and setter.
A null paramater is certainly much more user-friendly than passing -1 .
> 3) Regarding the returned class, CellRangeAddress: it appears that both
> AreaReference and CellRangeAddress have some limitations when it comes to
> handling Excel Version 97 versus 2007: both classes are a bit shaky when
> they are to decide if a cell range spans full rows/columns, as the different
> Excel versions support different maximum rows and columns.
> CellReference, which is used by both CellRangeAddress and AreaReference, has
> an inconspicuous comment which says :
> "...// TODO - "-1" is a special value being temporarily used for whole row
> and whole column area references. .."
> This is a quite informal specification for what I think is an important
> convention, as it offers a way to declare a whole row or column range in a
> spreadsheet version-independent way. Thus, I decided to stress this feature
> when using CellRangeAddress. However, the -1 convention seems to be
> implemented only in a sporadic selection of methods. E.g.
> CellRangeAddress.getNumberOfCells() returns wondrous results when operating
> with -1.
>
I see that this fix is in the patch. Thanks.
> 3.1) A side note on CellRangeAddress and AreaReference: it seems that the
> reference classes do not exploit the full power that a rich class hierarchy
> could offer:
> E.g. in various contexts, a CellRangeAdress (or AreaReference) parameter is
> not exactly what is permitted, but only an apporximation.
> An elaboorate reference class hierarchy could declare valid values more
> precisely. E.g.:
>
> CellSetRef <---+---- CellRangeRef <---------+----- CellRef
> | \ \
> | \ \
> +----- RowSetRef <----]\[-- RowRangeRef <--- RowRef
> | \
> | \
> +---ColumnSetRef <--- ColumnRangeRef <----- ColumnRef
>
Is CellSetRef a subclass of CellRangeAddress ?
I'd rather stay with current design and tighten it up to throw
IllegalArgumentException if a column range is passed instead of a row range,
etc.
You are welcome to uplaod a patch with (1) and (2).
Please feel free to re-open this ticket or create a new one .
Regards,
Yegor
--
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 53500] [Patch] Getter for repeating rows and columns
Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=53500
Joachim Herrmann <j....@web.de> changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|RESOLVED |REOPENED
Resolution|FIXED |---
--- Comment #3 from Joachim Herrmann <j....@web.de> ---
Hi Yegor,
I finally had time to deal with the setRepatingRowsAndColumns() method.
It is now relocated to class Sheet, and split into two methods that expect a
CellRangeAddress parameter, according to your suggestion.
Unfortunately, a bunch of additional get, add, and remove methods for the
HSSFName/XSSFName handling was needed, but as they are all package visible, the
user interface should not be to much cluttered by this.
Regards,
Joachim
--
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 53500] [Patch] Getter for repeating rows and columns
Posted by bu...@apache.org.
https://issues.apache.org/bugzilla/show_bug.cgi?id=53500
--- Comment #2 from Joachim Herrmann <j....@web.de> ---
Created attachment 29151
--> https://issues.apache.org/bugzilla/attachment.cgi?id=29151&action=edit
Patch file
--
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