Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Can't get .Dispose() to work in a foreach loop

So I've got this foreach loop here

foreach (string file in condensedFilesList)
{
    Image imgToAdd;
    imgToAdd = Image.FromFile(file);

    if (imgToAdd.Width < 1920 || imgToAdd.Height < 1080)
    {
        //neither of the commented out lines worked when placed here
        //imgToAdd = null;
        //imgToAdd.Dispose();
        condensedFilesList.Remove(file);
    }
    else
    {
        //neither of the commented out lines worked when placed here
        //imgToAdd = null;
        //imgToAdd.Dispose();
        continue;
    }
}

It contains a list of file paths pointing to .jpg images. About 80 of them of various sizes. I need the list to go through each image, check to see if its resolution is 1920*1080, and if it is not, remove that file path pointer from the array.

Right now it's going through, setting the image to review in the imgToAdd variable, then if the width property or height property don't match up that item is to be removed. This works for the first entry. It's resolution doesn't fit the bill, and my array will drop from 80 to 79 entries.

But I can't get my imgToAdd variable to empty out so I can assign it a new filePath. I keep running into a OutOfMemoryException. I've tried running .Dispose(), setting it equal to null, and I can't get it to actually empty itself of it's resources.

In the debugger .Dispose() causes imgToAdd to have a long list of errors in place of values when you inspect the element. All of it's properties are there, but valueless and replaced with errors. If I set it = to null, it works, and on the next iteration, imgToAdd = null. Buuuuut, I'm still getting OutOfMemoryException when it tries to assign a new filePath to the variable.

So I have no clue what's up with that. I'm hoping someone else could maybe point out what I'm doing wrong, I can't see it.

EDIT2 :

I'm just going to overwrite this edit space, if people want to check the evolution of the function as I update, hit up the edit history. I tried using a using(){} statement like @dlatikay recommended, and have it writing to a new list., but unfortunately I'm still getting the OutOfMemoryException. Here's the function right

        var tempList = new List<string>();

        foreach (string file in condensedFilesList)
        {
            using (Image imgToAdd = Image.FromFile(file))
            {
                if (imgToAdd.Width < 1920 || imgToAdd.Height < 1080)
                {
                    continue;
                }
                else
                {
                    tempList.Add(file);
                }
            }
        }

        condensedFilesList = tempList;
like image 269
Chris Avatar asked Feb 06 '23 13:02

Chris


1 Answers

Use using. And write the result to a new list, so you won't be modifying the source list while enumerating it:

var finalList = new List<string>();
foreach (string file in condensedFilesList)
{
    using(var imgToAdd = Image.FromFile(file))
    {
        if (imgToAdd.Width < 1920 || imgToAdd.Height < 1080)
        {
            /* omit */
        }
        else
        {
            finalList.Add(file);
        }
    }
}

No need to assign null, or to explicitely call Dispose(). I recommend to add try..catch, not all image files are valid.

like image 186
Cee McSharpface Avatar answered Feb 16 '23 00:02

Cee McSharpface