Is this correct for disposing an object using IDisposable
I have a class that implements the IDisposable interface. I am using a web client to download some data using AsyncDownloadString.
I am wondering if I have declared the event handlers correctly in the constructor and in the using statement of the web client? And is this correct way to remove event handlers in Dispose method?
Overrule is the correct way to use the IDisposable interface?
public class Balance : IDisposable
{
//Constructor
WebClient wc;
public Balance()
{
using (wc = new WebClient())
{
//Create event handler for the progress changed and download completed events
wc.DownloadProgressChanged += new DownloadProgressChangedEventHandler(wc_DownloadProgressChanged);
wc.DownloadStringCompleted += new DownloadStringCompletedEventHandler(wc_DownloadStringCompleted);
}
}
~Balance()
{
this.Dispose(false);
}
//Get the current balance for the user that is logged in.
//If the balance returned from the server is NULL display error to the user.
//Null could occur if the DB has been stopped or the server is down.
public void GetBalance(string sipUsername)
{
//Remove the underscore ( _ ) from the username, as this is not needed to get the balance.
sipUsername = sipUsername.Remove(0, 1);
string strURL =
string.Format("https://www.xxxxxxx.com",
sipUsername);
//Download only when the webclient is not busy.
if (!wc.IsBusy)
{
// Download the current balance.
wc.DownloadStringAsync(new Uri(strURL));
}
else
{
Console.Write("Busy please try again");
}
}
//return and display the balance after the download has fully completed
void wc_DownloadStringCompleted(object sender, DownloadStringCompletedEventArgs e)
{
//Pass the result to the event handler
}
//Dispose of the balance object
public void Dispose()
{
Dispose(true);
GC.SuppressFinalize(this);
}
//Remove the event handlers
private bool isDisposed = false;
private void Dispose(bool disposing)
{
if (!this.isDisposed)
{
if (disposing)
{
wc.DownloadProgressChanged -= new DownloadProgressChangedEventHandler(wc_DownloadProgressChanged);
wc.DownloadStringCompleted -= new DownloadStringCompletedEventHandler(wc_DownloadStringCompleted);
wc.Dispose();
}
isDisposed = true;
}
}
}
a source to share
This rule is correct, except that it wc
is placed twice, and GetBalance
will always be used wc
after it is placed!
Edit: version with this correction:
public class Balance : IDisposable
{
//Constructor
WebClient wc;
public Balance()
{
wc = new WebClient();
//Create event handler for the progress changed and download completed events
try {
wc.DownloadProgressChanged += new DownloadProgressChangedEventHandler(wc_DownloadProgressChanged);
wc.DownloadStringCompleted += new DownloadStringCompletedEventHandler(wc_DownloadStringCompleted);
} catch {
wc.Dispose();
throw;
}
}
~Balance()
{
this.Dispose(false);
}
//Get the current balance for the user that is logged in.
//If the balance returned from the server is NULL display error to the user.
//Null could occur if the DB has been stopped or the server is down.
public void GetBalance(string sipUsername)
{
//Remove the underscore ( _ ) from the username, as this is not needed to get the balance.
sipUsername = sipUsername.Remove(0, 1);
string strURL =
string.Format("https://www.xxxxxxx.com",
sipUsername);
//Download only when the webclient is not busy.
if (!wc.IsBusy)
{
// Download the current balance.
wc.DownloadStringAsync(new Uri(strURL));
}
else
{
Console.Write("Busy please try again");
}
}
//return and display the balance after the download has fully completed
void wc_DownloadStringCompleted(object sender, DownloadStringCompletedEventArgs e)
{
//Pass the result to the event handler
}
private bool isDisposed = false;
//Dispose of the balance object
public void Dispose()
{
if (!isDisposed)
Dispose(true);
GC.SuppressFinalize(this);
}
//Remove the event handlers
private void Dispose(bool disposing)
{
isDisposed = true;
if (disposing)
{
wc.DownloadProgressChanged -= new DownloadProgressChangedEventHandler(wc_DownloadProgressChanged);
wc.DownloadStringCompleted -= new DownloadStringCompletedEventHandler(wc_DownloadStringCompleted);
wc.Dispose();
}
}
}
a source to share
There are two correct ways to use the IDisposable object:
- put it in a block
using
OR - Wrap it in a class that also correctly implements IDisposable and removes it when you post this new class. Now you expect all instances of your new class to be created using blocks
using
.
Note that I said "or", not "and". Do this or the other, but not both.
Here, when you instantiate a WebClient with a block using
in the constructor, you are deleting it before you can ever use it elsewhere. In this case, you should only do option 2.
a source to share
Since you declared wc inside a using statement, you shouldn't be used outside of that. So I would assume your calls using wc inside GetBalance exceptions are being thrown. You must remove the using block from the balance constructor.
See " Using Statement (C # Reference) " for more information on the using statement.
a source to share
The other answers are correct, but they all missed the fact that you declare a finalizer when you don't want to.
Net Framework Design Guide (page 258):
- Avoid making types finalizable.
- Do make the type finalizable if the type is responsible for freeing unmanaged resources that do not have their own finalizer.
So, rpetrich's edited answer is correct, giving someone the rights to edit the finalizer.
a source to share