poi-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ST <st.mailingli...@gmail.com>
Subject Re: Bug 57450: Patch submitted, request for feedback
Date Mon, 09 Nov 2015 13:14:02 GMT
Javen,

Regarding your critical feedback given in your first email of this email
thread, I've updated my patch. But let's first have the other discussions
triggered by your second email of this email thread.

I actually like the tightly-coupled extreme of your vision: completely
hiding the tracker logic behind the Sheet API. Internally we could still
use a private class (same for all Sheet impls) to do the autosizing. I can
see the following benefits with this hide-behind-Sheet-API approach:
* "Solves" the problem of backward compatibility: same window-dependant
behaviour as before for SXSSFSheets, which can be fixed by using newly
added API (see below)
* Keeps same API for the different Sheet implementations
* Will still allow to exclude some rows from autosize calculation

I propose to add the following tracker API methods to the Sheet interface:
* trackRowForAutoSizing(Row)
* autoSizeAllColumnsByTrackedRows()
* autoSizeAllColumnsByTrackedRows(bool) // not sure if needed
These will complement the two existing autosize methods:
* autoSizeColumn(int) // keep this existing method
* autoSizeColumn(int,bool) // keep this existing method

What do you think about this?

The quicker but uglier solution would be to add the new tracker API only to
SXSSFSheet. But this in my opinion defeats the whole idea of the Sheet
interface since POI users will have to do the autosizing differently based
on the actual Sheet implementation.

I'll give it a try when I find the time.
  stefan.




On Wed, Nov 4, 2015 at 6:37 AM, Javen O'Neal <javenoneal@gmail.com> wrote:

> Here's what I had in mind about keeping support for
> SXSSFSheet.autoSizeColumn(int) and autoSizeColumn(int, bool):
>
> int rowAccessWindowSize = 1;
> SXSSFWorkbook wb = new SXSSFWorkbook(rowAccessWindowSize);
> SXSSFSheet sh = wb.createSheet();
> AutosizeColumnTracker tracker = sh.getAutosizeColumnTracker(); //this
> probably needs to be a singleton. Can be lazily-created.
> // tell workbook to keep track of column widths as they *might* be
> auto-sized at the end of the sheet
>
> boolean useMergedCells = false;
> tracker.monitorColumn(0, useMergedCells);
> tracker.monitorColumn(2, useMergedCells);
> tracker.monitorColumn(4); // equivalent to monitorColumn(4, true);
>
> // create a bunch of rows and cells, write some values to the cells, merge
> some cells across columns and rows so auto-size has something meaningful to
> do
> Row row;
> Cell cell;
> for (int r=0; r<=10; r++) {
>     row = sh.createRow(r);
>     for (int c=0; c<=10; c++) {
>         cell = row.createCell(0);
>         cell.setCellValue("row=" + r + ", column=" + c + ",
> somethingToMakeColumnsHaveDifferentWidth=" + ((int) Math.pow(r, c)));
>     }
> }
> // ... etc
>
> sh.autoSizeColumn(0); // okay
> sh.autoSizeColumn(0, false); // okay
> sh.autoSizeColumn(0, true); // throws exception: column widths were
> calculated using merged cell widths, but this pre-requisite is no longer
> valid.
> sh.autoSizeColumn(1); // throws exception: column was not monitored for
> auto-sizing
> sh.autoSizeColumn(2); // okay
>
> // and if we want to allow a user to auto-size a column that wasn't
> previously monitored, we might need to add an extra method that will
> indicate that the user understands that auto-sizing is computed solely from
> the row access window, and not to throw an UnmonitoredColumnForAutosize
> (insert your favorite name here) exception.
> autoSizeRowAccessWindowOnly = true;
> sh.autoSizeColumn(0, false, autoSizeRowAccessWindowOnly); // okay: makes
> existing functionality available
> // alternatively, if you want to allow the user to determine if a column
> gets autosized using merged cells or not at the time autoSizeColumn is
> called, the AutosizeColumnTracker would need to calculate and remember the
> widths for both cases (useMergedCells=true and useMergedCell=false).
>
>
> // finish up
> // ... etc
> wb.write(new FileOutputStream('example.xlsx'));
>
>
> I think the relationship between the tracker object and the SXSSFSheet
> warrants more discussion, as there are several ways to do this.
> Considerations:
> * extensibility: how can the AutosizeColumnTracker class be subclassed by
> users of POI? how can the SXSSFSheet be subclassed by users of POI? Should
> AutosizeColumnTracker be final or @Internal to avoid this question?
> * reusability: is there a need to create one autosize column tracker object
> that can be applied to multiple sheets (whether this is just the index of
> the monitored columns or also the column widths)?
> * if sh.autoSizeColumn uses the tracker, either the tracker needs to be
> passed in as a third argument or the sheet needs to know what tracker it is
> associated with. Every time a column goes outside the window, the
> SXSSFSheet needs to know which tracker object its associated with so it can
> update the column widths in the tracker (or maintain its own column
> widths). In your code, you require the user to explicitly call trackRow,
> but XSSFSheet and HSSFSheet don't have a similar construct: auto-sizing
> applies to ALL rows and this is not user configurable.
> * My vision of the AutosizeColumnTracker may be too tightly coupled with
> the SXSSFSheet. At the tightly-coupled extreme, AutosizeColumnTracker may
> be a private class (or just a private TreeMap member variable) inside of
> SXSSFSheet that maintains a TreeMap of tracked columns and column widths;
> all intents are registered directly on the SXSSFSheet:
> sxssfSheet.monitorColumnForAutosizing(0);
>
> Thoughts?
> Javen
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message