Disposing an ImageList

Question

What's the appropriate way to dispose an ImageList object?

Suppose I have some class with a private ImageList imageList member. Now, at some moment I perform the following code:

// Basically, lazy initialization.
if (imageList == null)
{
    imageList = new ImageList();
    Image[] images = Provider.CreateImages(...);
    foreach (var image in images)
    {
        // Does the 'ImageList' perform implicit copying here
        // or does it aggregate a reference?
        imageList.Images.Add(image); 

        // Do I need to do this?
        //image.Dispose();
    }
}

return imageList;

In the same class I have the Dispose method implementation, which is performed the following way:

public void Dispose()
{
    if (!disposed)
    {
        // Is this enough?
        if (imageList != null)
            imageList.Dispose();

        disposed = true;
    }
}

I'm sure there are some potential issues with this code, so could you please help me to make it correct.

Answer 1

Yes, it makes a copy. Note the CreateBitMap call below. Therefore, to keep your resource use as low as possible, you should uncomment your dispose line.

 private int Add(ImageList.Original original, ImageList.ImageCollection.ImageInfo imageInfo)
  {
    if (original == null || original.image == null)
      throw new ArgumentNullException("value");
    int num = -1;
    if (original.image is Bitmap)
    {
      if (this.owner.originals != null)
        num = this.owner.originals.Add((object) original);
      if (this.owner.HandleCreated)
      {
        bool ownsBitmap = false;
        Bitmap bitmap = this.owner.CreateBitmap(original, out ownsBitmap);
        num = this.owner.AddToHandle(original, bitmap);
        if (ownsBitmap)
          bitmap.Dispose();
      }
    }
    else
    {
      if (!(original.image is Icon))
        throw new ArgumentException(System.Windows.Forms.SR.GetString("ImageListBitmap"));
      if (this.owner.originals != null)
        num = this.owner.originals.Add((object) original);
      if (this.owner.HandleCreated)
        num = this.owner.AddIconToHandle(original, (Icon) original.image);
    }
    if ((original.options & ImageList.OriginalOptions.ImageStrip) != ImageList.OriginalOptions.Default)
    {
      for (int index = 0; index < original.nImages; ++index)
        this.imageInfoCollection.Add((object) new ImageList.ImageCollection.ImageInfo());
    }
    else
    {
      if (imageInfo == null)
        imageInfo = new ImageList.ImageCollection.ImageInfo();
      this.imageInfoCollection.Add((object) imageInfo);
    }
    if (!this.owner.inAddRange)
      this.owner.OnChangeHandle(new EventArgs());
    return num;
  }

When ImageList disposes, it disposes its copies of all the images. So again, yes, disposing of it when the form closes as you are is the right thing in addition to uncommenting your other dispose line.

protected override void Dispose(bool disposing)
{
  if (disposing)
  {
    if (this.originals != null)
    {
      foreach (ImageList.Original original in (IEnumerable) this.originals)
      {
        if ((original.options & ImageList.OriginalOptions.OwnsImage) != ImageList.OriginalOptions.Default)
          ((IDisposable) original.image).Dispose();
      }
    }
    this.DestroyHandle();
  }
  base.Dispose(disposing);
}