0

Is creating a new Pen in everytime Paint function called dangerous? Will it use high amount of ram?

So, which one should I use?

 private void Form1_Paint(object sender, PaintEventArgs e)
 {
     e.Graphics.DrawLine(new Pen(Color.Black.....

or

 tpen = new Pen(Color.Black)
 private void Form1_Paint(object sender, PaintEventArgs e)
 {
     e.Graphics.DrawLine(tpen
mucisk
  • 247
  • 5
  • 13
  • I think, second version is better. Paint event may occur very frequently, so there is no reason to create new Pen every time. – Andrey Korneyev Feb 06 '15 at 14:16
  • Dangerous? Yes, it may summon werewolves. High amount of ram? Yes, your machine will crash in seconds. On a more serious note: what do _you_ think? Have you done any research? Did you see [What happens if I don't call Dispose on the pen object?](http://stackoverflow.com/questions/4267729/what-happens-if-i-dont-call-dispose-on-the-pen-object)? Do you know what garbage collection is? – CodeCaster Feb 06 '15 at 14:18
  • Have you resolved your problem? – TaW Feb 09 '15 at 19:08

2 Answers2

3

  • If you don't change the Pen properties you can simply use Pens.Black, a standad Pen that you can't even dispose.

  • If you know that you will only use a few Pens you can keep them while the program runs.

  • If you need to create a large or unknown number of Pens, maybe to work through a file of drawing actions, then do create them in a using clause and don't worry about the time to create them; it happens very fast.

And diposing them is not so much about memory but about GDI resources, which are somewhat limited and therefore must not leak..

TaW
  • 53,122
  • 8
  • 69
  • 111
2

Pen implements IDisposable, which means that it should be disposed of as soon as it's not needed anymore. You should not create it directly within the DrwaLine call (since there's no way to dispose of it). It should not be a class member either, assuming it's only needed for the DrawLine call.

A better approach would be:

private void Form1_Paint(object sender, PaintEventArgs e)
{
    using(Pen pen = new Pen(Color.Black))
    {
        e.Graphics.DrawLine(tpen, ...);
    }
}

That way the pen is disposed of even if an exception occurs.

If all you're setting is the color, you can inline it by using the static Pens.Black property:

e.Graphics.DrawLine(Pens.Black, ...);

Since the object will be cached by the Pen class and you don't need to worry about disposing of it.

D Stanley
  • 149,601
  • 11
  • 178
  • 240
  • I agree; the benefit of creating a pen as a class field is small - but `using` is essential. In fact you very much should NOT create GDI objects and hang onto them! Create them, use them, dispose them. GDI objects may be limited. – Matthew Watson Feb 06 '15 at 14:19
  • @MatthewWatson GDI objects may be limited, but saying _"should NOT create GDI objects and hang onto them!"_ I think would depend on the type of object. `Pens` - sure; `Brushes` sure; `Fonts' maybe; `Metafiles` perhaps. `Bitmaps` depends. I would not want to keep recreating my offscreen bitmap, 30 FPS in my fancy flicker-free _scrolling chart control_. The **expense** in **recreating** the object may outweigh simply holding onto it. The only time I recreate it is if the control is resized. If the window is not visible or minimized, take a leaf out of DirectX/OpenGL and release them. –  Feb 06 '15 at 14:39
  • @MickyDuncan Yeah I meant brushes and pens and the ilk, not bitmaps (that would make double-buffering a bit challenging!) – Matthew Watson Feb 06 '15 at 15:28