2

I need to create an System.Net.Mail wrapper that I could use with IOC and in a Fluent way.

The idea would be to have two classes, Mailer and Mail, used as follows:

IMailer mailer = new Mailer();
IMail mail = new Mail()
  .Attach("Photo", fileData, "image/jpg")
  .Body("This is the photo I forgot to send")
  .From("from@xyz.com")
  .Subject("The missing photo")
  .To("to@xyz.com");
mailer.New(mail).Send();

Or in a completely Fluent way something like:

new Mailer()
  .New()
  .Attach("Photo", fileData, "image/jpg")
  .Body("This is the photo I forgot to send")
  .From("from@xyz.com")
  .Subject("The missing photo")
  .To("to@xyz.com")
  .Done()
  .Send();

I have done most of the work but I am having a problems with disposing. The mailer class:

  public interface IMailer : IDisposable {
    IMailer New(IMail mail);
    IMail New();
    void Cancel();
    void Send();
    void SendAsync(SendCompletedEventHandler callback, Object token = null);
  } // IMailer

  public class Mailer : IMailer {
    private SmtpClient _client;
    private IMail _mail;

    public Mailer() {
      _client = new SmtpClient();
    } // Mailer

    public IMailer New(IMail mail) {
      _mail = mail;
      return this;
    } // New

    public IMail New() {
      _mail = new Mail(this);
      return _mail;
    } // New

    public void Cancel() {
      _client.SendAsyncCancel();
    } // Cancel

    public void Send() {
      Send(null, null);
    } // Send

    public void SendAsync(SendCompletedEventHandler callback, Object token = null) {
      Send(callback, token);
    } // SendAsync

    private void Send(SendCompletedEventHandler callback = null, Object token = null) {

      using (MailMessage message = new MailMessage()) {
        message.From = new MailAddress(_mail.Data.From);
        message.Subject = _mail.Data.Subject;
        _mail.Data.To.ForEach(x => message.To.Add(new MailAddress(x)));
        message.Body = _mail.Data.Text;

        _mail.Data.Attachments.ForEach(x => { message.Attachments.Add(new Attachment(new MemoryStream(x.Value.Item2), x.Key, x.Value.Item1)); });

        if (callback == null)
          _client.Send(message);
        else {
          _client.SendCompleted += callback;
          _client.SendAsync(message, token);
        }

      };

    } // Send

    public void Dispose() {
      Dispose(true);
    } // Dispose

    protected virtual void Dispose(Boolean disposing) {
      if (disposing) {
        if (_client != null)
          _client.Dispose();
      }
    } // Dispose

  } // Mailer

And the mail class which contains the mail data and builds a mail is as follows:

  public interface IMail {
    MailData Data { get; }
    IMail Attach(String name, Byte[] file, String mime);
    IMail Body(String text);
    IMail From(String contact);
    IMail Subject(String subject);
    IMail To(String contact);
    IMailer Done();
  } // IMail

  public class Mail : IMail {
    private IMailer _mailer;
    public MailData Data { get; private set; }

    public Mail() {
      Data = new MailData();    
    } // Mail

    public Mail(IMailer mailer) {
      Data = new MailData();
      _mailer = mailer;      
    } // Mail

    public IMail Attach(String name, Byte[] file, String mime) {
      Tuple<String, Byte[]> attachment;
      if (!Data.Attachments.TryGetValue(name, out attachment))
        Data.Attachments.Add(name, new Tuple<String, Byte[]>(mime, file));
      return this;
    } // Attach

    public IMail Body(String text) {
      Data.Text = text;
      return this;
    } // Body

    public IMail From(String contact) {
      Data.From = contact;
      return this;
    } // From

    public IMail Subject(String subject) {
      Data.Subject = subject;
      return this;
    } // Subject

    public IMail To(String contact) {
      Data.To.Add(contact);
      return this;
    } // To

    public IMailer Done() {
      return _mailer;
    } // Done

  } // Mail  

  public class MailData {

    public Dictionary<String, Tuple<String, Byte[]>> Attachments { get; set; }
    public String From { get; set; }
    public String Subject { get; set; }
    public String Text { get; set; }
    public HashSet<String> To { get; set; }

    public MailData() {
      Attachments = new Dictionary<String, Tuple<String, Byte[]>>();
      To = new HashSet<String>();
    } // MailData

  } // MailData

The mailer uses a MailMessage which is disposed just after sending the email.

When I dispose the mailer then the SMTP Client is also disposed ...

However, I am not sure how to dispose the Mail itself.

This would be important to clear all those Attachements Files in the MailData dictionary.

I should be able to do it in two ways:

IMail mail = new Mail()
  .Attach("Photo", fileData, "image/jpg")
  // Define other properties of mail
// Send mail using mailer
mail.Dispose(); // Dispose mail

Or when completely fluently:

mailer
  .New()
  .Attach("Photo", fileData, "image/jpg")
  // Other properties of mail
  .Done()
  .Send()
  .Dispose();

This would dispose the mailer and its mail ... Or:

mailer
  .New()
  .Attach("Photo", fileData, "image/jpg")
  // Other properties of mail
  .Done()
  .Send()
  .Clear;

This would dispose the Mail associated to the mailer but not the mailer so I can send another mail with the same mailer.

Or any other configuration that you might suggest.

I am not sure the best way to go ...

Any advice is welcome.

Thank You,

Miguel

NOTE: when using IOC I will inject the IMailer in my services ...

Miguel Moura
  • 36,732
  • 85
  • 259
  • 481

2 Answers2

2

The only unmanaged resource I see in your code is SmtpClient. So only IMailer should be disposed. Mail and MailData are all managed so no need to dispose them. So you only have to call mailer.Dispose() and that's it.

(Btw it would be a good idea to use Mailer inside a using block.)

EDIT: I've just noticed that you also use SendAsnyc which makes disposing a bit trickier. When sending asynchronously you should call dispose in the callback. So your private Send method seems problematic as well because it disposes the MailMessage object (with the using block) but SendAsync might still need it.

See also:

Community
  • 1
  • 1
Vizu
  • 1,871
  • 1
  • 15
  • 21
  • Thank you for the tips about SmtpClient and Async. In relation to having the mailer inside a using block it makes sense but I am injecting the mailer into my services using StructureMap so SM will handle that in this case ... If I would created the mailer inside the services then it would make sense to wrap it into using ... Is this what you meant? – Miguel Moura Mar 25 '13 at 22:03
  • I don't know StructureMap but if I assume that it's similar to Unity (that I'm familiar with) then you have 2 options to get an object instance: either by injection (into a ctor or a property) or by resolving it directly (something like: var mailer = ObjectFactory.GetInstance()). In the latter case you can use a using block. In the former case (injection) I'm not sure about the right approach because you're responsible not only for disposing the object when you're finished with it but also for making sure that it gets disposed even when an exception happens. – Vizu Mar 26 '13 at 08:38
  • Another question: note that I am filling the attachments on MailMessage using a Stream. This is because it System.Net.Mail Attachment does not accept Byte[] which is what I have in my Mail object. Do I need to dispose the Streams? I am not sure if and how ... And they need to be present until the message is sent. – Miguel Moura Mar 26 '13 at 12:59
  • Check this: http://stackoverflow.com/questions/234059 The short answer is: you won't leak anything if you don't dispose a MemoryStream, but still you should dispose it to conform to the general rule of "dispose everything that implements IDisposable". If you dispose it then make sure that for the async version you dispose it in the completion callback. Man, this seemingly simple feature is not so straightforward at all. :) – Vizu Mar 26 '13 at 13:57
0

I'd create the class as a one-off; that is, one instance can send one email and then you throw it away. In this way you can properly dispose both the IMail and IMailer implementations when you call Send. You can configure your IoC to always return a new unique instance when you ask for IMailer.

This design has the added benefit that people using your interface won't forgot to dispose it.

Alternately, you could stick to the standard using block pattern, although it always seems odd to me to dispose something a container handed out.

Andy
  • 8,432
  • 6
  • 38
  • 76
  • But isn't creating a SMPT Client expensive? Shouldn't be a good idea to have one mailer by Thread? – Miguel Moura Mar 26 '13 at 12:57
  • It depends; you can do that but you still need to make sure it's disposed properly, so you have to know when a thread is shutting down and such. Unless the perf hit of creating an SMTP client is killing your app, I'd go for the option that ensures it will be disposed correctly as I think improper disposal is more likely to break your app then the "expensiveness" of creating the client. – Andy Mar 26 '13 at 21:42