none
Should you Dispose of Brush when using SystemColors.xxxx as the Color? RRS feed

  • Question

  • So, reading around varous places, (MSDN, these forums, CodeProject etc.) it's clear that you should dispose of any Pen's, Brushes etc. that you create and that perhaps the best way is via a Using block. But some behaviour has confused me and I think it might be related to the color I use when I create the Pen / Brush (read this in a post in the C# forums).

    It now works since I've used a Using block to replace the Dim....Dispose() shown below. But I don't understand why because all the Using block does is ensure that the used object is disposed of. It's just short-hand for Dim....Dispose() - at least that's what I understand.

    So if both versions cause Disposal of the Brush then why does one cause a crash and the other not? (I.e. this question is about understanding not solving the problem).

    The problem was (as far as I can tell) that when you tried to use the same code block a second time it crashed with an ArgumentException, complaining that there was an invalid parameter and the Exception detail seemed to suggest something (a parameter) was Nothing. This was maddening at first since I was using a Pen / Brush I had just created! At first I was chasing the other parameters until I had exhausted the possibilities and had to look at the Pen / Brush and the only clue was that C# post that suggested disposing of Pen's and Brushes created with SystemColors (and other built in classes) would cause problems. This would be equally weird since SystemColors is just a set of Static methods that return Color Structures

    Here's the type of code that was causing me an issue (and the working version) - had similar patterns in several places, all have been resolved with a switch to Using from Dim...Dispose().

    Class A
        Protected _ProgressColor As Color = SystemColors.Highlight       
    End Class
    
    Class B
        Inherits A
        'This version crashes
        Sub PaintBackground(e As PaintEventArgs)
            Dim sBrush As New SolidBrush(_ProgressColor)
            e.Graphics.DrawString(String, Font, sBrush, _BackgroundRect, New StringFormat With {.LineAlignment = StringAlignment.Center, .Trimming = StringTrimming.EllipsisWord, .Alignment = StringAlignment.Center})
            sBrush.Dispose
        End Sub
    
        'This version is fine
        Sub PaintBackground(e As PaintEventArgs)
            Using sBrush As New SolidBrush(_ProgressColor)
                e.Graphics.DrawString(String, Font, sBrush, _BackgroundRect, New StringFormat With {.LineAlignment = StringAlignment.Center, .Trimming = StringTrimming.EllipsisWord, .Alignment = StringAlignment.Center})
            End Using
        End Sub
    
    End Class

    Mike

    Friday, May 4, 2012 1:17 PM

All replies

  • Hi Mike,

    It seems to me that VB.Net may be doing the .Dispose before the DrawString action in the version which crashes.

    This may be only apparent at run-time, not sure what would happen if you stepped through the code.

    Therefore stick to using ( Using....End Using ) if that works for you. You can even nest them. >>

    Using pb As New PictureBox

    Using br As New SolidBrush(Color.White)

    'Some code here.

    End Using

    End Using

    '

    A similar thing happens when you have lots of code and then assign text to a TextBox or RichTextBox.

    The writing of text to the textbox can be the last thing that happens as apparently it is a CPU intensive operation,

    apparently DrawString is also CPU intensive.

    One way around this, as far as I remember, is to force a Refresh on the control you a drawing on.

    E.G:

    '

    Drawstring ..............

    PictureBox1.Refresh()

    'Then do the dispose.

    See if that works for you.  :)




    Regards,

    profile for John Anthony Oliver at Stack Overflow, Q&A for professional and enthusiast programmers

    Click this link to see the NEW way of how to insert a picture into a forum post.

    Installing VB6 on Windows 7

    App Hub for Windows Phone & XBOX 360 developers.

    Friday, May 4, 2012 1:43 PM
  • OK, so DrawString heads of into the bowels of Windows and GDI+, it takes a while, meanwhile my code keeps meerily going and Disposes of the brush then Windows / GDI+ tries to use it, only it isn't there anymore?

    So presumaly Using creates CIL that waits for Windows / GDI+ to say 'I'm done' then? Or something like that? What on earth does Refresh() do, force the pending DrawString to the top of the pile (would sound odd since intuitively you'd think it would just make the problem worse...)? (Should I stop asking...)

    Thanks,
    Mike

    Friday, May 4, 2012 2:06 PM
  • The dispose method of an object does not dispose (release) an object. It disposes the unmanaged resources from an object (standard behaviour if it is not overridden by somebody). 

    AFAIK do the drawing objects contain much handles which are among those unmanaged resources, that is the reason that it is advised to call that dispose method as soon as you are ready with it. The using keyword do that automatically for all objects which have implemented idisposable and the brush is one of those which implement that (about 80% of all windows forms classes). Not to prevent lack of memory but to prevent lack of handles.

    Therefore it is strange that the first crashes and the second does not, it is in fact the same code. 


    Success
    Cor


    Friday, May 4, 2012 2:19 PM
  • Hi Mike,

    Refresh causes an immediate update on the control you are writing to or drawing on.

    With what you say in your last post, it sounds like you could be right to me.  :)

    Try this in the code for a Button Click with one Button and one large RichTextBox on a Form.>>

    For num As Integer = 0 to 100000

    RichTextBox1.AppendText(num.ToString & Environment.NewLine)

    Next

    '

    As you can see you can get delays happening.

    Now you could write either of these just before the Next

    RichTextBox1.Refresh()

    'or

    RichTextBox1.ScrollToCaret()

    '

    'The Refresh would force an immediate update on the control, the ScrollToCaret would show the

    'last line written to the RichTextBox and therefore show the control updated too.

    '

    'Keep on asking as I am sure other forum users may offer better explanations.  :)




    Regards,

    profile for John Anthony Oliver at Stack Overflow, Q&A for professional and enthusiast programmers

    Click this link to see the NEW way of how to insert a picture into a forum post.

    Installing VB6 on Windows 7

    App Hub for Windows Phone & XBOX 360 developers.

    Friday, May 4, 2012 2:20 PM
  • @Cor - ok, so you're saying the call to dispose releases the handles that windows / GDI+ thinks it's using to do the actual drawing, hence things can go pear shaped? Rather than releasing the object. Makes sense apart from why the using Using instead of Dim...Dispose() doesn't suffer the same issue? Unless something likw that I suggested above is happening (only replace 'isn't there anymore' with 'doesn't have the necessary / expected handles')

    @John - thanks again, I haven't tried it, but I see exactly what you mean (and believe it) - interestingly then a 'bad' program could call Refresh() regularly and force its redraws to the top of the pile, de-prioritising everyone else's?

    Mike

    Friday, May 4, 2012 2:31 PM

  • @John - thanks again, I haven't tried it, but I see exactly what you mean (and believe it) - interestingly then a 'bad' program could call Refresh() regularly and force its redraws to the top of the pile, de-prioritising everyone else's?

    Mike

    Hi Mike,

    It depends on the intention of the original programmer, you may actually want to call Refresh to show a particular control being updated more than any other control deliberately. It could be a simple program where you wish to show continuous results. That is not always a 'bad' program, depending on your viewpoint.

    '

    If you lose the paint handle to a control, obviously no drawing is going to take place.

    You can do this in code too using RemoveHandler and the opposite is AddHandler

     I hope this helps you more.  :)




    Regards,

    profile for John Anthony Oliver at Stack Overflow, Q&A for professional and enthusiast programmers

    Click this link to see the NEW way of how to insert a picture into a forum post.

    Installing VB6 on Windows 7

    App Hub for Windows Phone & XBOX 360 developers.

    Friday, May 4, 2012 2:44 PM
  • There is no reason your first snippet of code would crash, and the second would not.

    Disposing of the brush is a quite normal thing to do. The 'drawing' isn't completing after the dispose: it's a linear progression of code. the 'using' construct just ensures the dispose method is called regardless of any actions within the Using/End Using.

    building a brush from a system color is no different than building it from any other color. A color is a structure (value). It will have no effect.

    I would suspect something else is going on; but it's not out of the realm of possibility that your other code has brought to the top an obscure bug/interaction. I find that unlikely, though. My question is: how is your 'PaintBackground' method getting called, and where did the parameter come from?

    The Using construct, however, does have a lot of overhead code under the hood: for it to ensure the 'using' object is disposed, there's a lot of catching of errors so that a call to 'dispose' can be made. Perhaps the Using construct is hiding an underlying error someplace else?

    Put it this way, with the code you have posted, there's no way to really replicate the problem.


    Stephen J Whiteley

    Friday, May 4, 2012 5:40 PM
    Moderator
  • Your 2 method does not generate the same IL code,

    those does

        Public Sub PaintBackground(ByVal e As PaintEventArgs)
            Dim sBrush As New SolidBrush(_ProgressColor)
            e.Graphics.DrawString("jjj", Form1.Font, sBrush, Form1.Bounds, New StringFormat With {.LineAlignment = StringAlignment.Center, .Trimming = StringTrimming.EllipsisWord, .Alignment = StringAlignment.Center})
            If sBrush IsNot Nothing Then
                sBrush.Dispose()
            End If
        End Sub


        Sub PaintBackground1(ByVal e As PaintEventArgs)
            Using sBrush As New SolidBrush(_ProgressColor)
                e.Graphics.DrawString("jjj", Form1.Font, sBrush, Form1.Bounds, New StringFormat With {.LineAlignment = StringAlignment.Center, .Trimming = StringTrimming.EllipsisWord, .Alignment = StringAlignment.Center})
            End Using
        End Sub

    And both will draw the text

    Not sure why this happen,... Unless you are not drawing in the paint event

    Friday, May 4, 2012 6:49 PM
  • It would be better to create one brush at the class level and reuse it. Mark the class with IDisposable and dispose of the brush in the Dispose method. If your code is in some class derived from Control then it will already implement IDisposable, which can be a pain. For forms you can dig around in the designer.vb file and place your dispose code inside the Dispose method there. An alternative is to use the Dispose event (I'm not sure if this is strictly correct!). At the moment you are creating the same color brush over and over which takes up one GDI handle and space on the heap for each object - a waste of resources, and you'll get more garbage collections.

    If you are using many instances of the class, then you could make the brush Shared, so that one instance of the brush is used for all the instances of your control / drawing widget thingy.

    Paint.Net is one of the best examples for drawing things - it uses a cache of brushes and pens, if you pick a new colour to draw with then it will look for an exising pen in the cache and only create it if required. (Of course you wouldn't want the cache to grow too big - there are many possible colours...)

    Friday, May 4, 2012 7:30 PM
  • The two code samples are equivalent.  The Using construct doesn't really do anything differently.  I doesn't wait.  In both cases, the call to .Dispose will not occur until after the call to DrawString returns.

    Can you clarify what "This version crashes" means?  What exception or error are you getting?  Where is the DrawString method being called? 

    Friday, May 4, 2012 9:47 PM
  • Really great that so many folk are interested and prepared to spend time on this, very grateful and I'd love to understand it, so I'll post the code tomorrow (UK time), it is in a method called from OnPaint but in another class (a component that does the painting for the control) - so it'll take me a bit of effort to extract the relevant bits of about 4 classes and a couple of interfaces. Please bear with me, I also need to rollback the changes to use Using! The error was an Argument Exception / Invalid Parameter, can't remember the detailed messages but the detail suggested a null / nothing for the brush.

    Mike

    Friday, May 4, 2012 10:27 PM
  • Selected code shown in all cases, happy to provide more if useful.

    It starts with an Abstract (MustInherit) class that inherits Control and then adds the necessary properties for a Progress Bar.

    Public MustInherit Class AbstractProgressBar
        Inherits Control
    
        Protected _BackgroundRect As RectangleF
        Protected _ProgressRect As RectangleF
    
        Protected Overrides Sub OnPaint(ByVal e As PaintEventArgs)
            MyBase.OnPaint(e)
            PaintBackground(e)
            PaintProgress(e)
            PaintText(e)
        End Sub
    
        Protected MustOverride Sub PaintBackground(ByVal e As PaintEventArgs)
        Protected MustOverride Sub PaintProgress(ByVal e As PaintEventArgs)
        Protected MustOverride Sub PaintText(ByVal e As PaintEventArgs)
    End Class

    Then there's a concrete implementation of a ProgressBar

    Public Class ProgressBar
        Inherits AbstractProgressBar
    
        Private _BackgroundPainter As IBackgroundPainter
        Private _ProgressPainter As IProgressPainter
    
        Public Sub New()
            _BackgroundPainter = New PlainBackgroundPainter
            _ProgressPainter = New PlainProgressPainter
        End Sub
    
        Protected Overrides Sub PaintBackground(ByVal e As PaintEventArgs)
            _BackgroundPainter.PaintBackground(_BackgroundRect, e)
        End Sub
    
        Protected Overrides Sub PaintProgress(ByVal e As PaintEventArgs)
            Dim drawProgress As Boolean = True
            If _Minimum < _Maximum Then
                If Progress = 0 Then drawProgress = False
            Else
                If Progress = 100 Then drawProgress = False
            End If
            If drawProgress Then
                _ProgressPainter.PaintProgress(MyBase._ProgressRect, e)
            End If
        End Sub
    
        Protected Overrides Sub PaintText(ByVal e As PaintEventArgs)
            'I thought since PaintEventArgs was passed ByVal it was a new instance that was a
            'copy of the original from OnPaint, but that once this method was finished
            'it would "automagically" have it's Dispose() method called 
    		
            Dim g As Graphics = e.Graphics
            Dim fontBrush = New SolidBrush(_TextColor)
            e.Graphics.DrawString(DisplayText(), _TextFont, fontBrush, _BackgroundRect, New StringFormat With {.LineAlignment = StringAlignment.Center, .Trimming = StringTrimming.EllipsisWord, .Alignment = StringAlignment.Center})
            fontBrush.Dispose() 'Issue here?
            g.Dispose() 'Once I realised that g was a reference to e.Graphics, not a new Graphics instance
            'I removed them all
        End Sub
    End Class

    IBackgroundPainter

    Public Interface IBackgroundPainter
        Inherits IDisposable
    
        Property BackgroundBorderWidth As Integer
        Property BackgroundBorderColor As Color
        Property BackgroundColor As Color
    
        Sub PaintBackground(ByVal box As RectangleF, ByVal e As PaintEventArgs)
        Sub PaintBackgroundBorder(ByVal box As RectangleF, ByVal e As PaintEventArgs)
    End Interface

    An Abstract BackgroundPainter

    Public MustInherit Class AbstractBackgroundPainter
        Inherits Component
        Implements IBackgroundPainter
    
        Protected MustOverride Sub PaintBackground(ByVal box As RectangleF, ByVal e As PaintEventArgs) Implements IBackgroundPainter.PaintBackground
        Protected MustOverride Sub PaintBackgroundBorder(ByVal box As RectangleF, ByVal e As PaintEventArgs) Implements IBackgroundPainter.PaintBackgroundBorder
    Ens Class

    Then a concrete implementation of a BackgroundPainter

    Public Class PlainBackgroundPainter
        Inherits AbstractBackgroundPainter
    
        Protected Overrides Sub PaintBackground(ByVal box As RectangleF, ByVal e As PaintEventArgs)
            Dim g As Graphics = e.Graphics
    
            'I've since moved the next 3 lines back to the Abstract Control in the OnPaint method,
            'before calling the PaintBacground etc. methods
            g.PageUnit = GraphicsUnit.Pixel 'Initial crashed were here, until I removed all g.Dispose() calls
            g.PixelOffsetMode = PixelOffsetMode.HighQuality
            g.SmoothingMode = SmoothingMode.AntiAlias
    
            Dim sBrush As New SolidBrush(_BackgroundColor) 'Then crash moved to here one I'd fixed the g.Dispose() calls, though the error description was essentially identical to that I saw above
    
            Select Case CornerStyle
                Case CornerStyle.Square
                    g.FillRectangles(sBrush, {box})
                Case CornerStyle.Angular
                    ...
                    g.FillRegion(sBrush, aRect.Region)
                    ...
                Case CornerStyle.Rounded
                    ...
                    g.FillRegion(sBrush, rRect.Region)
                    ...
            End Select
            sBrush.Dispose()
            g.Dispose()
            PaintBackgroundBorder(box, e)
        End Sub
    
        Protected Overrides Sub PaintBackgroundBorder(ByVal box As RectangleF, ByVal e As PaintEventArgs)
            If Not _BackgroundBorderWidth = 0 Then
                Dim pPen As New Pen(_BackgroundBorderColor, _BackgroundBorderWidth)
                Dim g As Graphics = e.Graphics
    
                Select Case CornerStyle
                    Case CornerStyle.Square
                        DrawSquareBorder(box, g, pPen)
                    Case CornerStyle.Angular
                        ...
                        g.DrawPath(pPen, aRect.Path)
                        ...
                    Case CornerStyle.Rounded
                        ...
                        g.DrawPath(pPen, rRect.Path)
                        ...
                End Select
                pPen.Dispose()
                g.Dispose()
            End If
        End Sub
    End Class

    What else is important to help work out what, why etc?

    This is the error:

    The Control .... has thrown an unhandled exception in the designer and has been disabled.
    
    Exception: Parameter is not valid.
    
    Stack trace:
      at
    ProgressBar.PaintBackground(PaintEventArgs e) in
        ProgressBar.vb:line 77
      at
    AbstractProgressBar.OnPaint(PaintEventArgs e) in
        AbstractProgressBar.vb:line 652

    If I run the application then I get the extra detail:

    System.ArgumentException was unhandled
      Message=Parameter is not valid.
      Source=System.Drawing
      StackTrace:
           at System.Drawing.Graphics.CheckErrorStatus(Int32 status)
           at System.Drawing.Graphics.DrawPath(Pen pen, GraphicsPath path)
           at PlainBackgroundPainter.PaintBackgroundBorder(RectangleF box, PaintEventArgs e) in PlainBackgroundPainter.vb:line 54
           at PlainBackgroundPainter.PaintBackground(RectangleF box, PaintEventArgs e) in PlainBackgroundPainter.vb:line 41
           at ProgressBar.PaintBackground(PaintEventArgs e) in ProgressBar.vb:line 77
           at ProgressBarBase.OnPaint(PaintEventArgs e) in AbstractProgressBar.vb:line 652
           at System.Windows.Forms.Control.PaintWithErrorHandling(PaintEventArgs e, Int16 layer)
           at System.Windows.Forms.Control.WmPaint(Message& m)
           at System.Windows.Forms.Control.WndProc(Message& m)
           at System.Windows.Forms.Control.ControlNativeWindow.OnMessage(Message& m)
           at System.Windows.Forms.Control.ControlNativeWindow.WndProc(Message& m)
           at System.Windows.Forms.NativeWindow.DebuggableCallback(IntPtr hWnd, Int32 msg, IntPtr wparam, IntPtr lparam)
           at System.Windows.Forms.UnsafeNativeMethods.DispatchMessageW(MSG& msg)
           at System.Windows.Forms.Application.ComponentManager.System.Windows.Forms.UnsafeNativeMethods.IMsoComponentManager.FPushMessageLoop(IntPtr dwComponentID, Int32 reason, Int32 pvLoopData)
           at System.Windows.Forms.Application.ThreadContext.RunMessageLoopInner(Int32 reason, ApplicationContext context)
           at System.Windows.Forms.Application.ThreadContext.RunMessageLoop(Int32 reason, ApplicationContext context)
           at Microsoft.VisualBasic.ApplicationServices.WindowsFormsApplicationBase.OnRun()
           at Microsoft.VisualBasic.ApplicationServices.WindowsFormsApplicationBase.DoApplicationModel()
           at Microsoft.VisualBasic.ApplicationServices.WindowsFormsApplicationBase.Run(String[] commandLine)
           at PBTest.My.MyApplication.Main(String[] Args) in 17d14f5c-a337-4978-8281-53493378c1071.vb:line 81
           at System.AppDomain._nExecuteAssembly(RuntimeAssembly assembly, String[] args)
           at System.AppDomain.ExecuteAssembly(String assemblyFile, Evidence assemblySecurity, String[] args)
           at Microsoft.VisualStudio.HostingProcess.HostProc.RunUsersAssembly()
           at System.Threading.ThreadHelper.ThreadStart_Context(Object state)
           at System.Threading.ExecutionContext.Run(ExecutionContext executionContext, ContextCallback callback, Object state, Boolean ignoreSyncCtx)
           at System.Threading.ExecutionContext.Run(ExecutionContext executionContext, ContextCallback callback, Object state)
           at System.Threading.ThreadHelper.ThreadStart()
      InnerException: 
    

    Now that kicks it off at a different line, line 54 but that's been called by 77, which was were the error sprung in the designer. The error detail is identical though. Here's the relevant lines:

    Class PlainBackgroundPainter
    'Line 54
    g.DrawPath(pPen, aRect.Path)
    
    'Line 41
    PaintBackgroundBorder(box, e)
    
    Class ProgressBar
    'Line 77
    _BackgroundPainter.PaintBackground(_BackgroundRect, e)
    
    Class AbstractProgressBar
    'Line 652
    PaintBackground(e)

    If I get rid of all those g.Dispose() lines then it works ok - so there must be a slight variation in there somewhere, I haven't managed to get the code back to exactly where it was, producing the same error on the lines that used Pen or Brush objects :-(

    So maybe it was all related to g.Dispose() calls and Pens and Brushes were never part of the problem? Difficult to tell if I can't reproduce it exactly.

    But the error is the same and I still don't understand why when passing ByVal, and disposing of g as the last call in the method it raises errors.  Would be interesting to know.

    Mike

    Saturday, May 5, 2012 4:32 PM
  • Hi,

    I think your issue is TOTALLY different form what your post at firstly. 

    All the judgement from members is depends upon without any other influence about using your first code. So your issue is by design rather than using g.dispose method. because change all g.dispose to using style will also caused this issue? Is it right? (you need to delete all the g.dispose code!)

    So instead of the issue about at first, How to use g.dispose seems to be more suitable. Please try to explain what the " Dimg AsGraphics =e.Graphics " mean? I can't understand this code.


    No code, No fact.


    • Edited by calanghei Monday, May 7, 2012 8:59 AM
    Monday, May 7, 2012 8:59 AM
  • Calenghei,

    I really don't see what you try to argument. Moreover, the Dim g as Graphics = e.Graphics is the most used sentence in .Net windows forms and graphics, the e.graphics holds the graphic object in the event arguments from an event which is handled.


    Success
    Cor

    Monday, May 7, 2012 1:02 PM
  • Hi Cor,

    >>I really don't see what you try to argument.

    sorry for that, and I'd like to some explanation. 

    OP the first question is talking about the difference from dispose and the using...end using block. As far as I know there are no difference between them. 

    OP show the code and the error. According to the error, I think this error is caused by abusing the parameter of the method (caused by the type of parameter). It has no relationship with the topic of this thread.


    No code, No fact.

    Tuesday, May 8, 2012 2:28 AM
  • Ah now I see what you mean, I thought he wanted to proof that the using keyword had nothing to do with his problem. 


    Be aware to dispose the brush which is created new in the method is of course exactly like it has to be, but not the graphic object which exist outside that method.


    Success
    Cor




    • Edited by Cor Ligthert Thursday, May 10, 2012 6:12 PM removed sentence about backgroundworker Mike explained was not the case
    Tuesday, May 8, 2012 6:45 AM
  • For reference.

    It wasn't a BackgroundWorker. Just a Class that was part of an Inheritance chain that started with one that Inherited Component. The Class is used to create an object which is a member of a Control (which Inherits from Control) and the PaintEventArgs is passed from the OnPaint event of the Control to the PaintBar method of the Component member. Pretty straightforward in principle.

    It looks like there was some other error in my code when I first posted the question, quite what I can't know since I can't back out all the changes to re-produce it. My original (but apparently wrong) sense that it was based on Using vs. Dim...Dispose must have been co-incident with fixing whatever that unknown error was - stupid of me to be doing multiple things at the same time, but hey, a lesson learnt.

    Ultimately, in the context of this discussion the Using pattern and the Dim...Dispose pattern do the same thing but consensus appears to favour Using so that's what I'll do in the future.

    The learning about not disposing of an object passed ByVal rather than one passed ByRef (which you probably wouldn't want to dispose of in most cases) is also very useful. Though I still don't really understand it since I thought ByVal meant you created a copy which had no link back the original. Maybe it's a shallow clone rather than a deep clone?

    I have no idea which thread(s) to mark as an answer though!

    Mike

    Thursday, May 10, 2012 5:34 PM
  • Mike,

    Thanks you explained it. 

    A brush is a (mutable) reference type, so using byref makes no sense. 

    "Byval" means for (mutable) reference types simply that the value of the reference is passed.



    Success
    Cor



    Thursday, May 10, 2012 6:11 PM
  • ...

    Ultimately, in the context of this discussion the Using pattern and the Dim...Dispose pattern do the same thing but consensus appears to favour Using so that's what I'll do in the future.

    The learning about not disposing of an object passed ByVal rather than one passed ByRef (which you probably wouldn't want to dispose of in most cases) is also very useful. Though I still don't really understand it since I thought ByVal meant you created a copy which had no link back the original. Maybe it's a shallow clone rather than a deep clone?

    I have no idea which thread(s) to mark as an answer though!

    Mike

    A lot of people are a big fan of using 'using': it really is a good way to go, and does prevent awkward bugs from cropping up.

    To clarify, the 'disposing' has nothing to do with byval and byref. There are a lot of resources which discuss the difference, and also go into the difference between value types and reference types (which are a different kettle of fish, but affected by byval and byref).

    Generally, an object which may require disposing is an reference type. You will only dispose of these objects if YOU create them. You will never dispose of objects which you didn't create. This has nothing to do with ByRef and ByVal.

    You can dispose of objects passed either way - as long as YOU have created them. However, this could lead to confusing code: if you think of a routine as 'you' then the programmer can follow the same pattern and be 'in the shoes' of the routine.

    For example, I, RoutineWork, have been given several variables (passed byref or byval - doesn't matter). Some of them do require disposing when they are finished with. However, I (RoutineWork), don't care. Did I create them? No, I was given them: it's not my job to tidy up these objects, I was just given them to use. I may have finished with them, but my calling routine may not have done so.

    If you create objects within a routine, then it is that routines job to correctly dispose of the objects. That is, when you are done with the object then dispose of it. Note, however, that the .NET runtime, in this case, is smart enough to know at the end of the routine that disposable objects can be disposed and it will do it automatically; calling the dispose method is not an absolute necessity (we won't get into when and how, but suffice to say, a lot of work has been done in this area to get the 'garbage collection' working optimally).

    ByRef and ByVal:

    This works different depending on whether you pass in a Value type or a reference type. This gives us 4 (four) combinations.

    ByVal and a Value type: the value is copied around. When you assign a variable to the value, you get a copy of the value. This works as expected, and there's very little mystery.

    ByVal and a Reference type: the reference type object is not copied per-se: a 'pointer' to that reference type is copied. You can manipulate this variable as much as you like, but any work performed on this variable (e.g. setting properties, calling methods) will act on the original object - you have a handy reference to the original object, even though you pass it ByVal.

    ByRef and a Value type: Similar to ByVal/Value, you pass a 'copy' of the value. However, if you reference the actual variable that is passed in ByRef, you actually have a pointer back to the original value. If you change the value, then the original variable in the calling form will be modified. This can get a little confusing, and is best understood by playing with it, and experimenting. Essentially, the ByRef allows you to pass values back to the calling method - in the C style languages, these are sometimes refered to as 'out' (or output) parameters, and is an alternative to returning values via a function. Returning values this way is not common, but is not so unusual that it's never used. An example is the TryParse methods on basic value types (e.g. Double.TryParse()) where the second, value type, parameter is passed ByRef.

    ByRef and Reference type: exactly the same as passing a value type. You can change the original reference type variable in the calling method to point to a completely different object (or even nothing). Obviously, a dangerous thing to do if the caller of said method doesn't know what the method will do to the object. You can both manipulate the original object and change the object pointed to by the calling method, similar to the ByRef/Value combination.

    That's a basic run down. Almost always you will pass variables (regardless of type) ByVal. You will need a very specific reason to pass a variable ByRef, and even then, you may be better off returning an object from a function rather than using ByRef. There are exceptions, of course and the above may not be strictly true for obscure cases, but I believe the above does describe the various scenarios.


    Stephen J Whiteley

    Monday, May 14, 2012 1:28 PM
    Moderator
  • I agree completely with SJWhiteley re byval/byref being unrelated to the topic, and I'm generally in agreement with his comment about disposing objects that you create and not disposing objects you don't.  But sometimes it is not so easy to know if you have dispose responsibilities.  Imagine a function that returns to you a Brush object, something like:

      MyBrush = GetMysteryBrush(...)

    You will use MyBrush only for the next few lines of code, and then you are done with it.  MyBrush is IDisposable.  The question is, should you dispose MyBrush or not?  The answer is that you can't possibly know unless you are instructed by the documentation of function GetMysteryBrush.  Maybe it creates a new object for you that you are supposed to dispose.  Or maybe it is handing to you one of its resources that you should not dispose.  If you call the constructor yourself, you know you created the object.  If you call a function that returns an object, it is not so clear.

    Also, a blanket rule like "If you create objects within a routine, then it is that routines job to correctly dispose of the objects" leave me cold.  Imagine a class that creates a collection of fonts during some phase of its operation.  That same routine will certainly not be the one that disposes them.  But I recognize that this rule will be right almost all the time, especially for a programmer who is a user of classes rather than a maker of classes for use by others.  In the OP's setting, the blanket rule is certainly correct.

    Monday, May 14, 2012 4:07 PM
  • ...

    Also, a blanket rule like "If you create objects within a routine, then it is that routines job to correctly dispose of the objects" leave me cold.  Imagine a class that creates a collection of fonts during some phase of its operation.  That same routine will certainly not be the one that disposes them.  But I recognize that this rule will be right almost all the time, especially for a programmer who is a user of classes rather than a maker of classes for use by others.  In the OP's setting, the blanket rule is certainly correct.

    A good point, but as you note, a general rule can apply to the OPs situation.

    The 'routine' can be literal and metaphorical: an object which creates disposable objects for use within the life of that object becomes, by definition, disposable. That object is responsible for disposal of those objects. If you create objects for use by others then I would think it's quite evident that you wouldn't dispose of them.

    If said disposable objects are used by a consumer and internally, things become much  more complicated: the object itself cannot dispose of them because the consumer may be using them, and the consumer cannot categorically dispose of them because object may be using them. This is where software architecture and documentation become critical.


    Stephen J Whiteley

    Monday, May 14, 2012 7:52 PM
    Moderator
  • ByRef and ByVal:

    This works different depending on whether you pass in a Value type or a reference type. This gives us 4 (four) combinations.

    ...
    An enormously useful section, many thanks.
    Monday, May 14, 2012 9:10 PM
  • MyBrush = GetMysteryBrush(...)

    I've had that problem several times whilst working with Path and region objects. Initially I was using a function to return a Path - as I needed to use the same code several times it made sense. But as you say if you have this arrangement then when do you dispose of what:

    Sub Paint(ByVal e As PaintEventArgs)
        Dim pPath as Path = GetPath(......)
        Dim g As Graphics = e.Graphics
        g.DrawPath(pPath)
        pPath.Dispose()
    Ens Sub
    
    Function GetPath(.....) As Path
        Dim TempPath as Path
        TempPath.AddArc(...)
        etc.
        ...
        Return TempPath
    End Function

    How does TempPath get disposed of? I first converted the Function to a Sub and sent the Path ByRef but then decided I didn't like that.

    I ended up creating a Class that had a Path as a member (and added several other useful features that the Function couldn't provide), made it IDisposable and just disposed of it every time I needed one...

    I know the GC would eventually get around to it but I have a preference to make sure my code is 'tidy' - then I know what I'm doing and am not reliant on the indeterminate GC.

    Very interesting discussion, thanks to all.

    Mike

    Monday, May 14, 2012 9:25 PM