1

I have an application that has to be able to send emails to users. I thought that creating SmtpClient every time a new email will be sent, like so using(smtpClient = new SmtpClient()) {// logic to send email} is redundant so I thought, I would create one instance of SmtpClient and reuse it throughout application lifecycle.

This is a helper class I created to do so, this class will have one instance per application and as a consequence the same instance of SmtpClient will be used all the time:

public class MailHelper : IMailHelper
{
    private readonly SmtpClient _smtpClient = new SmtpClient();

    public void SendMail(string recipient, string subject, string body, string attachmentContent = null, string attachmentName = null)
    {
        var mail = new MailMessage();
        mail.To.Add(new MailAddress(recipient));
        mail.Subject = subject;
        mail.Body = body;

        if (attachmentContent != null && attachmentName != null)
            mail.Attachments.Add(Attachment.CreateAttachmentFromString(attachmentContent, attachmentName));

        _smtpClient.Send(mail);
    }
}

Is it a good approach to do so? Do you see any pitfalls with it?

Mykhailo Seniutovych
  • 3,527
  • 4
  • 28
  • 50
  • If something implements `IDisposable` then it's usually recommended that you dispose of it. – Equalsk Jan 31 '18 at 15:17
  • `SmtpClient` implements the `IDisposable` interface. This means that it needs to be disposed, so the answer to your is probably no. It's not okay. – Zohar Peled Jan 31 '18 at 15:17
  • 3
    @ZoharPeled how do you explain HttpClient then? That is something that implements `IDisposable` that you should certainly not instantiate and dispose new instances for every request. – maccettura Jan 31 '18 at 15:19
  • @maccettura An IDisposable should be disposed as soon as possible. That doesn't mean you must HttpClient between requests.... – Zohar Peled Jan 31 '18 at 15:21
  • @ZoharPeled I think you may have forgotten a word in your comment, I am having trouble understanding. My previous comment refers to this known issue with using statements and HttpClient, see [here](https://aspnetmonsters.com/2016/08/2016-08-27-httpclientwrong/) for the details. – maccettura Jan 31 '18 at 15:23
  • Well at a fitst read it seems that HttpClient should not implement the IDisposable interface at all. Interesting. I did not know that. – Zohar Peled Jan 31 '18 at 15:47

0 Answers0