jferard / fastods

A very fast and lightweight (no dependency) library for creating ODS (Open Document Spreadsheet, mainly for Calc) files in Java. It's a Martin Schulz's SimpleODS fork
GNU General Public License v3.0
36 stars 6 forks source link

Incorrect values returned from TableCellWalker.getColumnCount() #244

Closed ideas-into-software closed 1 year ago

ideas-into-software commented 1 year ago

Attempt to obtain column count via TableCellWalker.getColumnCount() produces nondeterministic results.

Considering method name it’s Javadoc (“the current column count”), it would appear this method should return total number of columns present in a given sheet, as opposed to com.github.jferard.fastods.TableCellWalker.colIndex() (“the index of the cell in the current row”), i.e. in which column walker currently resides.

Perhaps TableCellWalker.getColumnCount() serves a different purpose, and it’s javadoc is misleading – kindly please clarify.

Also, perhaps related: TableCellWalker.hasNext() does not correspond to actual column count / it appears 'hasNext' returns true all the time.

Including @stbischof as per his request.

stbischof commented 1 year ago

@jferard could you please contact me via mail stefan.bischof@jena.de ?

jferard commented 1 year ago

Hi Stefan. sorry for being so late. You are right: the javadocs of these two methods are misleading. Under the hood, a row is an infinite list of cells. This infinite list has (obviously) an actual size: the index of the last used element plus one. That's what TableRow.getColumnCount returns. Since TableCellWalker inherits TableRow, the method TableCellWalker.getColumnCount() inherits the javadoc. I will fix this. Regarding TableCellWalker.hasNext: this method returns false if you use TableCellWalker.next without using TableCellWalker.set... before (eg. you create the walker and call next immediately.)

jferard commented 1 year ago

Regarding TableCellWalker.hasNext: this method returns false if you use TableCellWalker.next without using TableCellWalker.set... before (eg. you create the walker and call next immediately.)

This was true only for deprecated class RowCellWalkerImpl. TableCellWalker.hasNext always returns true. I deprecated hasNext/hasPrevious methods since they are basically useless.

I also renamed getColumnCount to getCurRowSize (more precisely: getColumnCount is deprecated in favor of getCurRowSize).