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" <[email protected]> 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 <[email protected]> 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 > > >
