Created attachment 21149 [details]
test case demonstrating the problem

The test case uses POI to write a simple 3x3 xls file and then read it back. 
getLastCellNum on each row in the re-read file returns the wrong value.

Created attachment 21364 [details]
Extended test case with the case reporetd in bug 43901

Notice that cellNum is '0' base. So if you create 3 cells, the lastCellNum has
to be 2.
I extended the testcase just to include the reported case.

Patch from Paolo to the test applied to svn

I agree with Paolo, this bug report is invalid, and POI is working as expected

I'm sorry to disagree, but the Javadoc for HSSFRow.getLastCellNum() says:

public short getLastCellNum()

    gets the number of the last cell contained in this row PLUS ONE.

    Returns:
        short representing the last logical cell in the row PLUS ONE, or -1 if
the row does not contain any cells.

I understand that cell numbers are zero based, but the documentation does say
PLUS ONE.  So if there are three cells, with numbers 0, 1 and 2, I would expect
that getLastCellNum() should return 3, not 2.

If you read a three-column XLS file created in Excel, you DO get 3 as the result
of getLastCellNum().

I still believe this is a bug.  Has it been fixed in the latest release?

(In reply to comment #5)
> I still believe this is a bug.  Has it been fixed in the latest release?


I admit I had to read the javadoc twice before I understood what this method did, but I am convinced the behaviour is correct and intentional.


This method allows standard 'zero based' array iteration (i=0; i<max; i++) over cells in a row.  I have used getLastCellNum() like this, POI internally uses getLastCellNum() like this, and I'm sure many other POI dependent projects have done the same.  So it's very unlikely that the behaviour can be changed.

The javadoc could be improved a bit. Here is a suggestion:

    /**
     * Gets the index of the last cell contained in this row <b>PLUS ONE</b>.  The result also 
     * happens to be the 1-based column number of the last cell.  This value can be used as a
     * standard upper bound when iterating over cells:
     * <pre> 
     * int maxColIx = row.getLastCellNum();
     * for(short colIx=0; colIx&lt;maxColIx; colIx++) {
     *   HSSFCell cell = row.getCell(colIx);
     *   if(cell == null) {
     *     continue;
     *   }
     *   //... do something with cell
     * }
     * </pre>
     * 
     * @return short representing the last logical cell in the row <b>PLUS ONE</b>, or -1 if the
     *  row does not contain any cells.
     */


I'm going to mark this bug as WORKSFORME.  If you still think there is a bug, could you please upload a junit test that shows it.

I agree with the functionality of getLastCellNum().  The specification is fine for me.

However the original report was that XLS files _written_ by POI have the wrong value for getLastCellNum().  If you write an XLS with three columns, then read it back, getLastCellNum() returns 2 not 3.  

If you read a XLS file with three columns generated by Excel, getLastCellNum() returns 3 as expected.

See the original attachment with this issue.

Our temporary workround was to modify HSSFRow.addCell to read:

    /**
     * used internally to add a cell.
     */

    private void addCell(HSSFCell cell)
    {
        short column=cell.getCellNum();
        if (row.getFirstCol() == -1)
        {
            row.setFirstCol(column);
        }
        if (row.getLastCol() == -1)
        {
            row.setLastCol((short) (column + 1));
        }

        if(column>=cells.length)
        {
          HSSFCell[] oldCells=cells;
          int newSize=oldCells.length*2;
          if(newSize<column+1) newSize=column+1;
          cells=new HSSFCell[newSize];
          System.arraycopy(oldCells,0,cells,0,oldCells.length);
        }
        cells[column]=cell;

        if (column < row.getFirstCol())
        {
            row.setFirstCol(column);
        }
        if (column >= row.getLastCol())
        {
            row.setLastCol((short) (column + 1));
        }
    }

(In reply to comment #7)
> I agree with the functionality of getLastCellNum().  The specification is fine
> for me.
> However the original report was that XLS files _written_ by POI have the wrong
> value for getLastCellNum().  If you write an XLS with three columns, then read
> it back, getLastCellNum() returns 2 not 3.  

You are right. Interestingly, if you re-save that file from Excel, getLastCellNum() returns 3, as expected.  So something is definitely wrong.

To reproduce the bug you don't even need to write-out/read-back the workbook.  You can just call getLastCellNum() after adding a cell:

    public void testLastCellNumIsCorrectAfterAddCell_bug43901(){
        HSSFWorkbook book = new HSSFWorkbook();
        HSSFSheet sheet = book.createSheet("test");
        HSSFRow row = sheet.createRow(0);

        // New row has last col -1
        assertEquals(-1, row.getLastCellNum());
        if(row.getLastCellNum() == 0) {
            fail("Identified bug 43901");
        }

        // Create two cells, will return one higher
        //  than that for the last number
        row.createCell((short) 0);
        assertEquals(1, row.getLastCellNum());
        row.createCell((short) 255);
        assertEquals(256, row.getLastCellNum());
    }

// 
It seems like there are a few related problems ( moving and removing cells too).  I'll upload a patch shortly to fix all of these.

Created attachment 21643 [details]
svn diff of changes to HSSFRow and test classes

Thanks for the patch Josh, applied to svn

*** Bug 43015 has been marked as a duplicate of this bug. ***

*** Bug 33317 has been marked as a duplicate of this bug. ***