Good point. I forgot about autosizing particular cells. This might be worthy of its own bug so as not to restrict/complicate the implementation of SXSSFSheet.
Consider two possible scenarios: 1) user wants to autosize two columns using just the header text in one or two rows, 2) user wants to autosize data in the rows under headers. If the user adds new rows to the spreadsheet after tracking existing data rows, are the newly added rows also tracked? Basically, tracked-by-default and not-tracked-by-default behavior. Do you want to support both workflows in POI, or require the user to resolve this? On 9 Nov 2015 3:37 p.m., "ST" <[email protected]> wrote: > 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" <[email protected]>: > > > 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 > > > > > > > > > >
