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 23:36:59 GMT
By having the possibility of tracking specific rows only, one gains the
capability to exclude some rows from autosizing calculations. This is
useful independently of type of Sheet. Example usecase is a row of column
headings. This was also proposed by the issue reporter, see his latest
comment [1]. Although not strictly related to the SXSSF autosizing bug, it
would be a nice side-effect to gain.

[1] https://bz.apache.org/bugzilla/show_bug.cgi?id=57450#c4
Den 9 nov 2015 17:30 skrev "Javen O'Neal" <javenoneal@gmail.com>:

> Tracking is simply unnecessary for XSSFSheet and HSSFSheet because the rows
> never fall outside the memory window. Keep in mind that tracking rows uses
> a little more memory and performs some expensive sizing logic that may not
> get used. I'd keep the autosizing tracker at the SXSSF level since it only
> applies for windowed memory sheets.
>
> I expect users needing to write themselves into the SXSSFSheet API when it
> differs from the Sheet interface, and this is certainly one of those cases.
> On 9 Nov 2015 05:14, "ST" <st.mailinglists@gmail.com> wrote:
>
> > 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