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

Worksheet Enumerator does not behave as expected #1829

Open
OssianEPPlus opened this issue Jan 29, 2025 · 0 comments
Open

Worksheet Enumerator does not behave as expected #1829

OssianEPPlus opened this issue Jan 29, 2025 · 0 comments
Assignees
Labels
bug Something isn't working enhancement New feature or request

Comments

@OssianEPPlus
Copy link
Contributor

OssianEPPlus commented Jan 29, 2025

You can currently delete worksheets while iterating through the same worksheets. This causes issues as the worksheet originally at index [1] gets locked to index 0.

Per standard iteration of e.g. List we should throw if the collection is changed while iterating through it.
We should also add a RemoveAll() function to Worksheets with a predicate similar to List

[TestMethod]
public void DeletingWorksheetsWithParameters()
{
    using (var p = OpenPackage("DeletingGroupOfWorksheets.xlsx"))
    {
        var wb = p.Workbook;
        var worksheets = wb.Worksheets;

        for (int i = 0; i < 5; i++)
        {
            worksheets.Add($"Data {i}");
        }

        for (int i = 0; i < 5; i++)
        {
            worksheets.Add($"SomeWorksheet{i}");
        }

        //Deleting worksheets with foreach
        //Skips over index[1]
        //Should probably throw as the enumerator changes while in it
        foreach (var ws in p.Workbook.Worksheets)
        {
            if (ws.Name.StartsWith("Data ", StringComparison.OrdinalIgnoreCase))
            {
                p.Workbook.Worksheets.Delete(ws);
            }
        }

        var countWs = p.Workbook.Worksheets.Count();

        Assert.AreEqual(countWs , 5);
    }

Worksheets.Delete() on RemoveAndShift appears to cause the skipping.

Ideally users should be able to do something like Remove all or Delete all :

p.Workbook.Worksheets.RemoveAll(x => x.Name.StartsWith("Data "));
@OssianEPPlus OssianEPPlus self-assigned this Jan 29, 2025
@OssianEPPlus OssianEPPlus added bug Something isn't working enhancement New feature or request labels Jan 29, 2025
JanKallman added a commit that referenced this issue Feb 5, 2025
JanKallman added a commit that referenced this issue Feb 6, 2025
* Fix for #1829

* Fixed an issue with 1-based worksheet collections
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants