[adn258]

So I have a project that I'm working on and I'm a little paranoid now about all this dispose stuff; so I have a a timer within another form I open from the main form; this form has a timer. If I just call this.Dispose(); on the closing event of the form will that dispose of the timer? I have been reading online that it doesn't always dispose and that you should remove it's event etc. like

 private void Connection_Error_FormClosed(object sender, FormClosedEventArgs e)
        {
            msgtime.Stop();
            msgtime.Tick -= new EventHandler(msgtime_Tick);
            msgtime.Dispose();
            this.Dispose();
        }

Is that acceptable and should you be that scared of these things in C# or what's the deal?

[adn258]

So I was looking on MSDN there's the whole Idisposible method you have to place the dispose method within the class and all that jazz so it disposes if disposed is called;

When should this be used though? I thought it was smarter to just create normal classes and let the framework auto dispose and garbage collect the class?

That said below I use an abstract class I created for sending email for gmail and yahoo in a program I created. Is this class acceptable? Is that coding ok or should it be Idisposible? How do you know? Should I be using objects this way and is my code below acceptable?

Thanks guys

public abstract class Mail
    {
        protected string strUsername;
        protected string strPassword;
        protected string strEmail;

        public abstract void SendMessageOut(string strSubject, string strBody);
    }

    public class Gmail : Mail
    {
        public Gmail(string email, string username,string password)
        {
           strUsername = username; strPassword = password; strEmail = email;
        }

        public override void SendMessageOut(string strSubject, string strBody)
        {
            using (SmtpClient sc = new SmtpClient())
            {
                sc.Host = "smtp.gmail.com"; sc.Port = 587; sc.Timeout = 20000; sc.EnableSsl = true; sc.Credentials = new NetworkCredential(strUsername, strPassword);
                sc.Send(strEmail, strEmail, strSubject, strBody);
            }

        }
    
    }

    public class Yahoo : Mail
    {
        public Yahoo(string email, string username, string password)
        { strUsername = username; strPassword = password; strEmail = email; }

        public override void SendMessageOut(string strSubject, string strBody)
        {
            using (SmtpClient sc = new SmtpClient())
            {
                sc.Host = "smtp.mail.yahoo.com"; sc.Port = 465; sc.EnableSsl = true; sc.Timeout = 20000; sc.Credentials = new NetworkCredential(strUsername, strPassword);
                sc.Send(strEmail, strEmail, strSubject, strBody);
            }
        }
    }

[AdamSpeight2008]

Usually when you hold on to some form of OS resource / handle. Implement IDisposable

[Nakor]

You might implement it in a situation where you want to make sure that a resource is being released as soon as it is no longer needed. One example would be closing the connection to a database once the connection is no longer needed. IDisposable is required if you want to be able to use your class with a using block.

// more code

// DisposableResource implements IDisposable
using (var someResouce = new DisposableResource()) 
{
    // Any resource/connection being made available by
    // the DisposableResource class will be available in this block


} // Any Connection/Resources are now released

// more code

That being said, I don't see where you class would need to implement IDisposable. Any resources being used by the SmtpClient, which already implements the IDisposable interface, are being disposed of once the code leaves the using block. However, if you just wanted to you could do something like this maybe.

    public abstract class Mail : IDisposable
    {
        private SmtpClient _client;
        protected string strUsername;
        protected string strPassword;
        protected string strEmail;

        public SmtpClient Client { get { return _client; } }

        public abstract void SendMessageOut(string strSubject, string strBody);

        public Mail(string email, string username, string password, SmtpClient client)
        {
            this._client = client;
            strUsername = username; 
            strPassword = password; 
            strEmail = email;
        }

        public void Dispose()
        {
            _client.Dispose();
        }
    }

    public class Gmail : Mail
    {
        public Gmail(string email, string username, string password)
            : base(email, username, password, new SmtpClient())
        { }

        public override void SendMessageOut(string strSubject, string strBody)
        {
            Client.Host = "smtp.gmail.com";
            Client.Port = 587;
            Client.Timeout = 20000;
            Client.EnableSsl = true;
            Client.Credentials = new NetworkCredential(strUsername, strPassword);
            Client.Send(strEmail, strEmail, strSubject, strBody);
        }
    }

    public class Yahoo : Mail
    {
        public Yahoo(string email, string username, string password)
            : base(email, username, password, new SmtpClient())
        { }

        public override void SendMessageOut(string strSubject, string strBody)
        {
            Client.Host = "smtp.mail.yahoo.com";
            Client.Port = 465;
            Client.EnableSsl = true;
            Client.Timeout = 20000;
            Client.Credentials = new NetworkCredential(strUsername, strPassword);
            Client.Send(strEmail, strEmail, strSubject, strBody);
        }
    }

Which could then be used in the following way.

using (var yahoo = new Yahoo("email", "user", "password"))
{
    yahoo.SendMessageOut("subject", "body");
}

This post has been edited by Nakor: 13 July 2012 - 10:29 PM

[Momerath]

If the timer is part of the controls collection it will automatically be disposed.

[adn258]

Nakor, on 13 July 2012 - 09:27 PM, said:

You might implement it in a situation where you want to make sure that a resource is being released as soon as it is no longer needed. One example would be closing the connection to a database once the connection is no longer needed. IDisposable is required if you want to be able to use your class with a using block.

// more code

// DisposableResource implements IDisposable
using (var someResouce = new DisposableResource()) 
{
    // Any resource/connection being made available by
    // the DisposableResource class will be available in this block


} // Any Connection/Resources are now released

// more code

That being said, I don't see where you class would need to implement IDisposable. Any resources being used by the SmtpClient, which already implements the IDisposable interface, are being disposed of once the code leaves the using block. However, if you just wanted to you could do something like this maybe.

    public abstract class Mail : IDisposable
    {
        private SmtpClient _client;
        protected string strUsername;
        protected string strPassword;
        protected string strEmail;

        public SmtpClient Client { get { return _client; } }

        public abstract void SendMessageOut(string strSubject, string strBody);

        public Mail(string email, string username, string password, SmtpClient client)
        {
            this._client = client;
            strUsername = username; 
            strPassword = password; 
            strEmail = email;
        }

        public void Dispose()
        {
            _client.Dispose();
        }
    }

    public class Gmail : Mail
    {
        public Gmail(string email, string username, string password)
            : base(email, username, password, new SmtpClient())
        { }

        public override void SendMessageOut(string strSubject, string strBody)
        {
            Client.Host = "smtp.gmail.com";
            Client.Port = 587;
            Client.Timeout = 20000;
            Client.EnableSsl = true;
            Client.Credentials = new NetworkCredential(strUsername, strPassword);
            Client.Send(strEmail, strEmail, strSubject, strBody);
        }
    }

    public class Yahoo : Mail
    {
        public Yahoo(string email, string username, string password)
            : base(email, username, password, new SmtpClient())
        { }

        public override void SendMessageOut(string strSubject, string strBody)
        {
            Client.Host = "smtp.mail.yahoo.com";
            Client.Port = 465;
            Client.EnableSsl = true;
            Client.Timeout = 20000;
            Client.Credentials = new NetworkCredential(strUsername, strPassword);
            Client.Send(strEmail, strEmail, strSubject, strBody);
        }
    }

Which could then be used in the following way.

using (var yahoo = new Yahoo("email", "user", "password"))
{
    yahoo.SendMessageOut("subject", "body");
}

Right I don't think this is necessary +1 for everyone's answer and I get why you need to use it if a resource needs to be released right away databases and all that jazz what I don't get is you using part of my class you have a field that's private in this code example i.e. and smtp client and you use a get accessor in the property to set itself as the _client which is a field and is already set as part of the object constructor? Why does this help you? You could just use _client instead of the SmtpClient which wouldn't help you at all if you're setting that field in the constructor?

AT LEAST not as far as I see I'm probably wrong so someone tell me why that's necessary again I just like to know everything.

[Nakor]

It's generally good practice to keep fields private to the class and to use properties to access those values intead. Using the Client property with only a get accessor makes the SmtpClient available for use without making it available for modification. This helps to ensure that your class will only be used in the way you've designed it.

However, with that being said it was late and I was probably over complicating things, it happens. The way you have yours is most likely fine for your situation.

But just for fun, here's another way you could implement your classes that would make use of the IDisposable automatically. Inherit from the SmtpClient class directly rather than using it as a dependency.

    public class GmailClient : SmtpClient
    {
        public GmailClient(string username, string password) : base()
        {
            Host = "smtp.gmail.com";
	    Port = 587;
	    Timeout = 20000;
	    EnableSsl = true;
	    Credentials = new NetworkCredential(username, password);
        }
    }

And it could still be used in the following way.

using (var gmail = new GmailClient("user", "pass"))
{
    gmail.Send("from", "to", "subject", "body");
}

Of course there's probably a hundred (at least) different ways you could go about this.

Now, back to why I was passing in the SmtpClient through the constructor rather than just instantiating it inside the class is that it makes the class extensible without the need for modification.
If I created a class that was dependent upon the SmtpClient as yours was but passed the dependency in through the constructor (known as dependency injection) then my class is now able to make use of not only the SmtpClient class but also any class that inherits from the SmtpClient class, such as the GmailClient class above. Let's say you wanted to implement a Mailer class that could send mail using SmtpClient, GmailClient, or YahooClient, you wouldn't need to include all 3 of those classes in your Mailer class.

    public class Mailer
    {
        private SmtpClient _client;

        public Mailer(SmtpClient client)
        {
            this._client = client;
        }
        
        public void Send(string from, string to, string subject, string body)
        {
            using (_client)
            {
                _client.Send(from, to, subject, body);
            }
        }
    }

Now since we are passing in the SmtpClient through the constructor, we can set our internal _client object to whatever type of SmtpClient gets passed into the constructor, SmtpClient, GmailClient, or YahooClient. This is called polymorphism and is a very powerful feature of object oriented programming.

Now you could use the Mailer class in the following way

SmtpClient client = new SmtpClient();
GmailClient gClient = new GmailClient("user", "pass");
YahooClient yClient = new YahooClient("user", "pass");

Mailer mailer = new Mailer(client);
mailer.Send("from", "to", "subject", "body");

mailer = new Mailer(gClient);
mailer.Send("from", "to", "subject", "body");

mailer = new Mailer(yClient);
mailer.Send("from", "to", "subject", "body");

If in the future you wanted to create more versions of SmtpClient, say for hotmail or some other mail service you would not need to modify your mailer class at all to make use of it.

This post has been edited by Nakor: 14 July 2012 - 06:37 AM

[tlhIn`toq]

Remember that .Dispose() as the signature indicates is a method.

You should make a method in your class/form to be executed when .Dispose()d of. That way you can do things like unsubscribe from events.

Event subscriptions take a minute amount of memory. But they do take memory. Not unsubscribing will cause a memory leak. If you do it a lot within the running of your application it can add up.

Your two threads for .Dispose() have been merged. In the future please don't open multiple threads for the same thing.

[adn258]

tlhIn`toq, on 14 July 2012 - 06:34 AM, said:

Remember that .Dispose() as the signature indicates is a method.

You should make a method in your class/form to be executed when .Dispose()d of. That way you can do things like unsubscribe from events.

Event subscriptions take a minute amount of memory. But they do take memory. Not unsubscribing will cause a memory leak. If you do it a lot within the running of your application it can add up.

Your two threads for .Dispose() have been merged. In the future please don't open multiple threads for the same thing.

Thanks friend. Why not just use the OnFormClosing though to unsubscribe the events like for the timer? Also are you counting the timer itself or just the even it has? Also when you say unsubscribe events do you mean ALL the events in that form even the buttons and stuff? Sorry I'm very confused?

[[email protected]]

As Momerath suggested, if you are using the WinForms timer, and have dragged and dropped the timer onto the form, or manually created the timer and passed the form's components container to the timer's constructor (or have otherwise added it to the form's components container), it will be disposed of automatically when the form is disposed of, as the form's Dispose(bool) method calls Dispose() on the components container.

When is the form disposed of?

If it's your main form (the/a form started with Application.Run()), or if you show a form using Show(), Dispose() will be called automatically for you when the form is closed.

If you show the form modally using ShowDialog(), then Dispose() is not called automatically on close; you have to call it yourself manually. Similarly, if the form is hidden, and part of a multiple-document interface application, calling close on it will not result in a call to Dispose().

Luckily, the Form class (indirectly) implements IDisposable, so in the case of showing the form modally, just do the standard thing, and use a using statement to make sure Dispose() is called on the Form instance:

using (Connection_Error dialog = new Connection_Error()) {
    dialog.ShowDialog(this);
}

In the case of the MDI application scenario, you just have to remember to call Dispose() if you close a hidden form.


Back to the timer...

If the timer isn't part of the components container, then yes, it is a good idea to call Dispose() on the timer, just to clear up it's resources deterministically, and as early as possible, and thus avoid the penalties of finalization. If you don't call Dispose() on such a timer, its unmanaged resources will be eventually freed by the timer object's finalizer, of which the garbage collector will eventually call.


As for the event subscriptions...

I'm sure I've talked about this before in one of your threads, but there is technically no need to un-subscribe from an event when the event publisher won't ever (now , or in the future) be in scope longer than the event listener.

Event subscriptions do take up memory, of course, but it's managed memory that the garbage collector will collect. The only time event subscriptions pose a problem in terms of memory leaks is when the publishing object is in scope longer than the listening object, as the listening object will fall out of scope, but won't be eligible for garbage collection (when it should be,) because the publishing object is still in scope, and still has a reference to it through an event subscription, hence you get a so called 'managed memory leak'.

This is clearly not the case in your example. The Connection_Error form instance is the listening object (it is listening to the Tick event of the timer), and the Timer instance is the publisher (it publishes the Tick event).

The Form object contains the Timer object; therefore, the Timer object will be in scope no longer than the form is in scope.

The only way a problem would be caused by not un-subscribing from the Tick event is if the timer object was to be in scope longer than the form object, thus meaning the form object was kept alive by the reference the timer object has to the form through it's Tick event, when it should be garbage collected. This clearly isn't the case here, as when the form falls out of scope, so does the timer, meaning there is no great need to un-subcribe from the Tick event, and both objects become eligible for garbage collection.

Quote

Also when you say un-subscribe events do you mean ALL the events in that form even the buttons and stuff

Perfect case in point. Do you have to manually un-subscribe from every single GUI control event you subscribe to? Of course not. That would be a massive pain. Why don't you have to? Because the GUI controls are the event publisher objects, the Form is the listening object, and the controls will not be in scope longer than the form, because the form contains the controls, so when the form falls out of scope, so do all it's controls, meaning the form and its controls are both eligible for garbage collection, and so no 'memory leaks' occur from any event subscriptions.

EDIT:

Where should you dispose of things in your form?

The OnFormClosed method seems a reasonable place to do it.

Although, I would say the most logical place to dispose of things is in the overridden Dispose(bool) method, which is located in the Designer.cs file of the form.

This post has been edited by [email protected]: 15 July 2012 - 06:03 AM

[adn258]

Amazingly well written post thank you codingsupernatural guy . Well explained.

This post has been edited by adn258: 15 July 2012 - 01:32 PM