Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
daniel-shuy opened this issue Mar 22, 2022 · 6 comments

Comments

@daniel-shuy
Copy link

daniel-shuy commented Mar 22, 2022

StreamingWorkbook#getSheetIndex(String) and StreamingWorkbook#getSheet(String) are case sensitive:

@Override
public Sheet getSheet(String name) {
int index = getSheetIndex(name);
if(index == -1) {
throw new MissingSheetException("Sheet '" + name + "' does not exist");
}
return reader.getSheets().get(index);
}

@Override
public int getSheetIndex(String name) {
return findSheetByName(name);
}

int findSheetByName(String name) {
for(int i = 0; i < reader.getSheetProperties().size(); i++) {
if(reader.getSheetProperties().get(i).get("name").equals(name)) {
return i;
}
}
return -1;
}

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 daniel-shuy changed the title StreamingWorkbook#getSheet(String) behavior is inconsistent with poi-ooxml StreamingWorkbook#getSheetIndex(String) and StreamingWorkbook#getSheet(String) behavior is inconsistent with poi-ooxml Mar 22, 2022
@daniel-shuy
Copy link
Author

Oh wow that was fast, thanks @pjfanning!

@daniel-shuy
Copy link
Author

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

@pjfanning
Copy link
Contributor

pjfanning commented Mar 23, 2022

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
Copy link
Author

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

@pjfanning
Copy link
Contributor

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
Copy link
Author

@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!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants