Is using try/finally a good practice for memory management?

Question

One of my senior told me to use try/ finally block for all the methods to clear the initialized objects data. for eg:

var serviceProxy = new NotificationServiceProxy();
try
{
     return serviceProxy.GetNotifications(userID, request.Filters, request.FilterProperty, usertypeid);
}
finally
{
     serviceProxy = null;
}

Is that a good practice? if i'm using try/ catch for all my methods to clear the initialized objects data.

Answer 1

Not in C#, use using instead:

using(var serviceProxy = new NotificationServiceProxy()){
    serviceProxy.GetNotifications(userID, request.Filters, request.FilterProperty, usertypeid);
    // Do stuff with the service proxy
}

// serviceProxy has now been cleaned up

This is the pattern to ensure that IDisposable variable get cleaned up.

For instances that aren't disposable this isn't needed - just let them pass out of scope and leave it to the GC.

If NotificationServiceProxy uses lots of resources and you need to be sure that it is finalised correctly then make it disposable and always wrap it in a using or in another class that also implements IDisposable. If it doesn't then this try-finally pattern is a waste of time.

Answer 2

Yes, if you need to dispose a variables.

Answer 3

You don't need to clear a local variable. Before you exit the method, the stack will be reverted anyway (which will free up the space on the stack used by the variable) and setting the local variable to null will not free up space on the heap (the garbage collector will do that). Use try finally if you need to do some cleanup, for example object disposal or closing files.

Answer 4

No that's not a good practice.

• You are doing work manually, the compiler/garbage collector will do for you. This is unnecessary time spent and unnecessary code clutter you need to remove or maintain later.

• You completely missed to .Dispose() the service reference. Although this could be done in a finally block, the using block exists exactly for this purpose.

Answer 5

Is it a good practice or not, it depends on the scenarios. It's often good to catch the exception so that you can handle unexpected cases gracefully. If your program has developed some other ways to handle exception cases then Try/Finally block should be suffice since Catch block have some performance penalties.

Please review the Consideration When Using Try Catch article from MSDN.

Answer 6

First of all, you have no need to clear the local variables (Garbage Collector will do it for you), however, it's safe:

 Object o = new Object(); 
 ...
 o = null; // safe, but not required

What you have to do is to clear unmanaged resources; typical way for this is implementing IDisposable interface and wrapping corresponding instances into using:

 // Providing that serviceProxy implements IDisposable
 using (var serviceProxy = new NotificationServiceProxy()) {
   return serviceProxy.GetNotifications(
     userID, 
     request.Filters, 
     request.FilterProperty, 
     usertypeid);
 } // <- here .Net will call serviceProxy.Dispose() and free the resources allocated

sometime you have to restore the initial state, and that is the case try..finally has been designed for:

  Cursor savedCursor = Cursor.Current;

  try {
    Cursor.Current = Cursors.WaitCursor;
    ... 
  }
  finally {
    // Rain (exception thrown) or shine (no exception), please, restore my cursor
    Cursor.Current = savedCursor;
  }

In your particular case, I can't see any state to restore, that's why try..finally is not the pattern that should be used. However, it's quite possible that serviceProxy allocates some unmanaged resources (say, TCP/IP port, connection or alike) so it seems that you should implement IDisposable for NotificationServiceProxy class (if it's not imlemented yet) and wrap the instance into using.

Answer 7

This makes sense when you need something that HAS to happen like Disposing IDisposable objects whether your operation has failed or succeeded.

try
{
    myDisposable.Do();
}
finally
{
    myDisposable.Dispose(); 
}

you have a built in mechanism which does exactly that : Using

using(IDisposable myDisposable = new Something())
{
    myDisposable.Do();
}

Now has to good practice :

That's kinda up to your error handling strategy and cleanup needs :

• If i have something that HAS to be done whether the operation succeed or failed i would put a try finally block.

• if i have something that only need error handling and no cleanup i would place try catch without a finally block

• If i have to do some kind of error handling i would also place a catch block in there

• If i wan't to make some error handling there like a log and then have the Exception propagate from that stack i would also re throw the exception.

or any permutation of the above conditions ..

For instance :

 try
 {
     myDisposable.Do();
 }
 catch(Exception e)
 {
     Log("Error at something something",e); 
     throw e;
 }
 finally
 {
     myDisposable.Dispose(); 
 }

I for one do not place try catch finally every where, i want the places in my code that which do contain them stand out stating that these are operations that are known to go wrong for some known reason.

i would welcome new exceptions i'm not yet aware of and handle them accordingly.