[Bug 53500] [Patch] Getter for repeating rows and columns

[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