Bug 63749; Make getLastRowNum() and getFirstRow() return -1 instead of 0 on empty...

git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1871589 13f79535-47bb-0310-9956-ffa450edef68

diff --git a/src/java/org/apache/poi/hssf/usermodel/HSSFSheet.java b/src/java/org/apache/poi/hssf/usermodel/HSSFSheet.java
index c47cc1c6accbf9d96fb2ecef26a24ebd366732ce..f398c914cf93544b3493ca3be326641c3f20859b 100644 (file)
--- a/src/java/org/apache/poi/hssf/usermodel/HSSFSheet.java
+++ b/src/java/org/apache/poi/hssf/usermodel/HSSFSheet.java
@@ -114,8 +114,8 @@ public final class HSSFSheet implements org.apache.poi.ss.usermodel.Sheet {
     protected final InternalWorkbook _book;
     protected final HSSFWorkbook _workbook;
     private HSSFPatriarch _patriarch;
-    private int _firstrow;
-    private int _lastrow;
+    private int _firstrow = -1;
+    private int _lastrow = -1;
 
     /**
      * Creates new HSSFSheet   - called by HSSFWorkbook to create a sheet from
@@ -316,6 +316,12 @@ public final class HSSFSheet implements org.apache.poi.ss.usermodel.Sheet {
                 _firstrow = findFirstRow(_firstrow);
             }
             _sheet.removeRow(hrow.getRowRecord());
+
+            // if there are no more rows, then reset first/last
+            if(_rows.size() == 0) {
+                _firstrow = -1;
+                _lastrow = -1;
+            }
         }
     }
 

diff --git a/src/java/org/apache/poi/ss/usermodel/Sheet.java b/src/java/org/apache/poi/ss/usermodel/Sheet.java
index 517cbec3530f9b101968b4d8194e61afbde8b487..54b4f4303e8575af2854381ee674b5442a1b1bbe 100644 (file)
--- a/src/java/org/apache/poi/ss/usermodel/Sheet.java
+++ b/src/java/org/apache/poi/ss/usermodel/Sheet.java
@@ -99,6 +99,7 @@ public interface Sheet extends Iterable<Row> {
      * than expected!
      *
      * @return the number of the first logical row on the sheet (0-based)
+     *      or -1 if no row exists
      */
     int getFirstRowNum();
 
@@ -110,7 +111,7 @@ public interface Sheet extends Iterable<Row> {
      * method will include such rows and thus the returned value might be higher
      * than expected!
      *
-     * @return last row contained on this sheet (0-based)
+     * @return last row contained on this sheet (0-based) or -1 if no row exists
      */
     int getLastRowNum();
 

diff --git a/src/ooxml/java/org/apache/poi/xssf/streaming/SXSSFSheet.java b/src/ooxml/java/org/apache/poi/xssf/streaming/SXSSFSheet.java
index 2139111f5d097898da045439ddf1b89e2bfb2acc..f639dc12e7b20f76ecb38a11d8be4741ddd5f5c1 100644 (file)
--- a/src/ooxml/java/org/apache/poi/xssf/streaming/SXSSFSheet.java
+++ b/src/ooxml/java/org/apache/poi/xssf/streaming/SXSSFSheet.java
@@ -206,7 +206,7 @@ public class SXSSFSheet implements Sheet
         if(_writer.getNumberOfFlushedRows() > 0) {
             return _writer.getLowestIndexOfFlushedRows();
         }
-        return _rows.size() == 0 ? 0 : _rows.firstKey();
+        return _rows.size() == 0 ? -1 : _rows.firstKey();
     }
 
     /**
@@ -217,7 +217,7 @@ public class SXSSFSheet implements Sheet
     @Override
     public int getLastRowNum()
     {
-        return _rows.size() == 0 ? 0 : _rows.lastKey();
+        return _rows.size() == 0 ? -1 : _rows.lastKey();
     }
 
     /**

diff --git a/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFSheet.java b/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFSheet.java
index dcbe55cbd211bfb82cc98b4466795f76868979a3..58fe33134b0d7cae4e2af1db5ce5b9330930760e 100644 (file)
--- a/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFSheet.java
+++ b/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFSheet.java
@@ -1098,7 +1098,7 @@ public class XSSFSheet extends POIXMLDocumentPart implements Sheet  {
      */
     @Override
     public int getFirstRowNum() {
-        return _rows.isEmpty() ? 0 : _rows.firstKey();
+        return _rows.isEmpty() ? -1 : _rows.firstKey();
     }
 
     /**
@@ -1220,7 +1220,7 @@ public class XSSFSheet extends POIXMLDocumentPart implements Sheet  {
         // A test with 1_000_000 rows shows that querying getLastRowNum with lastKey() implementation takes ~40 ms,
         // and ~1.2 ms with cached implementation. 40 ms is negligible compared to the time of evaluation a million
         // cells, and the lastKey implementation is much more elegant and less error prone than caching.
-        return _rows.isEmpty() ? 0 : _rows.lastKey();
+        return _rows.isEmpty() ? -1 : _rows.lastKey();
     }
 
     @Override

diff --git a/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFWorkbook.java b/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFWorkbook.java
index b0a5474040670e0af86e54d99a4d80af36d8a41d..67da55b3a531a8fe94db5bdbaa0f4d08e9d265a8 100644 (file)
--- a/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFWorkbook.java
+++ b/src/ooxml/testcases/org/apache/poi/xssf/usermodel/TestXSSFWorkbook.java
@@ -113,8 +113,8 @@ public final class TestXSSFWorkbook extends BaseTestXWorkbook {
         assertEquals(1, wb1.getSheetAt(0).getLastRowNum());
         assertEquals(0, wb1.getSheetAt(1).getFirstRowNum());
         assertEquals(0, wb1.getSheetAt(1).getLastRowNum());
-        assertEquals(0, wb1.getSheetAt(2).getFirstRowNum());
-        assertEquals(0, wb1.getSheetAt(2).getLastRowNum());
+        assertEquals(-1, wb1.getSheetAt(2).getFirstRowNum());
+        assertEquals(-1, wb1.getSheetAt(2).getLastRowNum());
 
         File file = TempFile.createTempFile("poi-", ".xlsx");
         OutputStream out = new FileOutputStream(file);
@@ -151,8 +151,8 @@ public final class TestXSSFWorkbook extends BaseTestXWorkbook {
         assertEquals(1, wb2.getSheetAt(0).getLastRowNum());
         assertEquals(0, wb2.getSheetAt(1).getFirstRowNum());
         assertEquals(0, wb2.getSheetAt(1).getLastRowNum());
-        assertEquals(0, wb2.getSheetAt(2).getFirstRowNum());
-        assertEquals(0, wb2.getSheetAt(2).getLastRowNum());
+        assertEquals(-1, wb2.getSheetAt(2).getFirstRowNum());
+        assertEquals(-1, wb2.getSheetAt(2).getLastRowNum());
 
         sheet1 = wb2.getSheetAt(0);
         assertEquals(1.2, sheet1.getRow(0).getCell(0).getNumericCellValue(), 0.0001);

diff --git a/src/testcases/org/apache/poi/hssf/usermodel/TestBugs.java b/src/testcases/org/apache/poi/hssf/usermodel/TestBugs.java
index 46056ff82e7d02f5e0bbec057dc255103742bc80..b0aa06cb4e3a2cfe2ed0f34f0d710e5d0c38d0c9 100644 (file)
--- a/src/testcases/org/apache/poi/hssf/usermodel/TestBugs.java
+++ b/src/testcases/org/apache/poi/hssf/usermodel/TestBugs.java
@@ -1230,9 +1230,9 @@ public final class TestBugs extends BaseTestBugzillaIssues {
         HSSFWorkbook wb = new HSSFWorkbook();
         HSSFSheet s = wb.createSheet();
 
-        // No rows, everything is 0
-        assertEquals(0, s.getFirstRowNum());
-        assertEquals(0, s.getLastRowNum());
+        // No rows, first/last return -1
+        assertEquals(-1, s.getFirstRowNum());
+        assertEquals(-1, s.getLastRowNum());
         assertEquals(0, s.getPhysicalNumberOfRows());
 
         // One row, most things are 0, physical is 1

diff --git a/src/testcases/org/apache/poi/ss/usermodel/BaseTestRow.java b/src/testcases/org/apache/poi/ss/usermodel/BaseTestRow.java
index 6e663c37ee39b79b53bc5172610909f61863a9a0..1634330138c49da1839295bb78b2cd02119f1cef 100644 (file)
--- a/src/testcases/org/apache/poi/ss/usermodel/BaseTestRow.java
+++ b/src/testcases/org/apache/poi/ss/usermodel/BaseTestRow.java
@@ -532,4 +532,32 @@ public abstract class BaseTestRow {
             assertNull(f1);
         }
     }
+
+    @Test
+    public void testLastRowEmptySheet() {
+        Workbook wb = _testDataProvider.createWorkbook();
+        Sheet sheet = wb.createSheet("sheet1");
+
+        assertEquals("Sheet without rows should return -1 as lastRowNum",
+                -1, sheet.getLastRowNum());
+        Row row = sheet.createRow(0);
+        assertNotNull(row);
+
+        assertEquals("Sheet with one row should return 0 as lastRowNum",
+                0, sheet.getLastRowNum());
+    }
+
+    @Test
+    public void testFirstRowEmptySheet() {
+        Workbook wb = _testDataProvider.createWorkbook();
+        Sheet sheet = wb.createSheet("sheet1");
+
+        assertEquals("Sheet without rows should return -1 as firstRowNum",
+                -1, sheet.getFirstRowNum());
+        Row row = sheet.createRow(0);
+        assertNotNull(row);
+
+        assertEquals("Sheet with one row should return 0 as firstRowNum",
+                0, sheet.getFirstRowNum());
+    }
 }

diff --git a/src/testcases/org/apache/poi/ss/usermodel/BaseTestSheet.java b/src/testcases/org/apache/poi/ss/usermodel/BaseTestSheet.java
index 657442026fefbba169d9d5b4eac62c2cf972dac4..02930a99593b21a2f2559017e74198a90414d142 100644 (file)
--- a/src/testcases/org/apache/poi/ss/usermodel/BaseTestSheet.java
+++ b/src/testcases/org/apache/poi/ss/usermodel/BaseTestSheet.java
@@ -130,8 +130,8 @@ public abstract class BaseTestSheet {
         Workbook workbook = _testDataProvider.createWorkbook();
         Sheet sheet1 = workbook.createSheet();
         assertEquals(0, sheet1.getPhysicalNumberOfRows());
-        assertEquals(0, sheet1.getFirstRowNum());
-        assertEquals(0, sheet1.getLastRowNum());
+        assertEquals(-1, sheet1.getFirstRowNum());
+        assertEquals(-1, sheet1.getLastRowNum());
 
         Row row0 = sheet1.createRow(0);
         assertEquals(1, sheet1.getPhysicalNumberOfRows());
@@ -139,8 +139,8 @@ public abstract class BaseTestSheet {
         assertEquals(0, sheet1.getLastRowNum());
         sheet1.removeRow(row0);
         assertEquals(0, sheet1.getPhysicalNumberOfRows());
-        assertEquals(0, sheet1.getFirstRowNum());
-        assertEquals(0, sheet1.getLastRowNum());
+        assertEquals(-1, sheet1.getFirstRowNum());
+        assertEquals(-1, sheet1.getLastRowNum());
 
         sheet1.createRow(1);
         Row row2 = sheet1.createRow(2);

diff --git a/src/testcases/org/apache/poi/ss/usermodel/BaseTestXEvaluationSheet.java b/src/testcases/org/apache/poi/ss/usermodel/BaseTestXEvaluationSheet.java
index cd60b3f6ff2888128cf146c8fe3b04c3e7dab2f4..f21842b4ec342170955a004eadf12199cc9717cd 100644 (file)
--- a/src/testcases/org/apache/poi/ss/usermodel/BaseTestXEvaluationSheet.java
+++ b/src/testcases/org/apache/poi/ss/usermodel/BaseTestXEvaluationSheet.java
@@ -36,7 +36,7 @@ public abstract class BaseTestXEvaluationSheet {
         Sheet underlyingSheet = sheetPair.getKey();
         EvaluationSheet instance = sheetPair.getValue();
 
-        assertEquals(0, instance.getLastRowNum());
+        assertEquals(-1, instance.getLastRowNum());
 
         underlyingSheet.createRow(0);
         underlyingSheet.createRow(1);