monitorjbl / excel-streaming-reader

An easy-to-use implementation of a streaming Excel reader using Apache POI
Apache License 2.0
943 stars 342 forks source link

StreamingWorkbook#getSheetIndex(String) and StreamingWorkbook#getSheet(String) behavior is inconsistent with poi-ooxml #248

Open daniel-shuy opened 2 years ago

daniel-shuy commented 2 years ago

StreamingWorkbook#getSheetIndex(String) and StreamingWorkbook#getSheet(String) are case sensitive: https://github.com/monitorjbl/excel-streaming-reader/blob/9e2f0202baa663041837c26aa5ae278d31d7c9d6/src/main/java/com/monitorjbl/xlsx/impl/StreamingWorkbook.java#L103-L110 https://github.com/monitorjbl/excel-streaming-reader/blob/9e2f0202baa663041837c26aa5ae278d31d7c9d6/src/main/java/com/monitorjbl/xlsx/impl/StreamingWorkbook.java#L67-L70 https://github.com/monitorjbl/excel-streaming-reader/blob/9e2f0202baa663041837c26aa5ae278d31d7c9d6/src/main/java/com/monitorjbl/xlsx/impl/StreamingWorkbook.java#L29-L36

This behavior is inconsistent with poi-ooxml, where all Workbook#getSheetIndex(String) and Workbook#getSheet(String) implementations are case insensitive, eg. XSSFWorkbook#getSheetIndex(String) and XSSFWorkbook#getSheet(String):

    /**
     * Returns the index of the sheet by his name (case insensitive match)
     *
     * @param name the sheet name
     * @return index of the sheet (0 based) or {@code -1} if not found
     */
    @Override
    public int getSheetIndex(String name) {
        int idx = 0;
        for (XSSFSheet sh : sheets) {
            if (name.equalsIgnoreCase(sh.getSheetName())) {
                return idx;
            }
            idx++;
        }
        return -1;
    }
    /**
     * Get sheet with the given name (case insensitive match)
     *
     * @param name of the sheet
     * @return XSSFSheet with the name provided or {@code null} if it does not exist
     */
    @Override
    public XSSFSheet getSheet(String name) {
        for (XSSFSheet sheet : sheets) {
            if (name.equalsIgnoreCase(sheet.getSheetName())) {
                return sheet;
            }
        }
        return null;
    }
daniel-shuy commented 2 years ago

Oh wow that was fast, thanks @pjfanning!

daniel-shuy commented 2 years ago

@pjfanning just realized its for your fork 😅 , could you create a PR here as well?

pjfanning commented 2 years ago

this is not my repo and my PRs here have been not been merged in recent past - I didn't fork this project for a laugh

daniel-shuy commented 2 years ago

Is this repo no longer maintained? @pjfanning is that why you forked the project?

pjfanning commented 2 years ago

create a PR yourself and see if it is accepted - I no longer create PRs on this repo - my fork is frequently maintained and its API remains very similar to this one

daniel-shuy commented 2 years ago

@pjfanning I see that the last PR and reply from the owner is from 2021, it seems like its no longer maintained. I'll use your fork then, thanks!