1

I have the following code:

using System.Drawing;
...
Graphics _graphics = Graphics.FromImage(...)
...
public void Draw(Stream stream, Rectangle rect, Color color) {
    _graphics.FillRectangle(new SolidBrush(Color), 0, 0, Width, Height);
    _graphics.DrawImage(new Bitmap(stream), rect);
}

Should I surround the drawImage with using on new Bitmap(...)? Same question on the new SolidBrush(...)

Soner Gönül
  • 97,193
  • 102
  • 206
  • 364
taminov
  • 591
  • 7
  • 23
  • Yes, I think you should. I would also do that for unmanaged resources. It might allow the GC to dispose of them earlier. – Elad Lachmi Dec 25 '12 at 09:44

2 Answers2

4

Yes, you should wrap them in using statements. Also you should ensure that the Dispose methods is invoked on the _graphics instance that you are using in this class. This could be done by having the containing class implement IDisposable so that the consumer of this class could wrap it in a using statement.

Darin Dimitrov
  • 1,023,142
  • 271
  • 3,287
  • 2,928
2

Yes, disposing these resources is important. Particularly a bitmap is troublesome, it consumes lots of unmanaged memory to store the pixel data but the managed Bitmap class wrapper is very small. You can create a lot of Bitmap objects before triggering a garbage collection, giving high odds that the Bitmap constructor will start failing because there is no more unmanaged memory left.

So rewrite it like this:

public void Draw(Stream stream, Rectangle rect, Color color) {
    using (var bmp = new Bitmap(stream))
    using (var brush = new SolidBrush(Color)) {
        _graphics.FillRectangle(brush, 0, 0, Width, Height);
        _graphics.DrawImage(bmp, rect);
    }
}
Hans Passant
  • 922,412
  • 146
  • 1,693
  • 2,536