2

I have an email helper class that has a method that sends email:

private static class EmailHelper
{
    public static void SendEmail(string from, string to, string subject, string message)
    {
        MailMessage msg = new MailMessage(from, to, subject, body);
        var client = new SmtpClient("my.smpthost.com"); //this will be moved to config.
        client.Send(msg);
    }
}

My smtp host string will be placed in my config file. Question is what's the recommended way to pass the smtp host to this method?

  1. Should I have a constructor in this class that takes in the smtp string?
  2. Or, should I pass the smtp as a 5th parameter?
  3. Or should this method directly read from the config?

(I realize that there may be several different ways to do this, but I am trying to understand the recommended approach, or at least approaches that are NOT recommended, keeping in mind the SOLID principles.)

  • 1
    I'm not sure what problem this method is even solving. If you make it store the host, you're just wrapping SmtpClient with less options. – Magus Aug 15 '14 at 20:14
  • This question is too broad, SOLID principles contain a lot of recommended ways, and no answer will be "the best" to answer your questions Maybe rephrasing or breaking this question into multiple questions would improve it. – Roman Mik Aug 15 '14 at 20:24
  • @Magus This question is not about this method actually...for any method like this one where it is dependent on a configuration item (in this case, the SMTP host), I'm wondering how to tell this method what that configuration item is? Should this method read directly from the config, or should it be passed in? – Prabhu Aug 15 '14 at 20:39
  • Well in that case, the answer is D: They should be passed in. – Magus Aug 15 '14 at 20:59
  • 1
    That is totally dependent on use-case. If you plan to send several things through the same client in a single session, it belongs in a constructor. If it is meant to be different per call, it belongs in the method. But in this case, my original answer applies. – Magus Aug 15 '14 at 21:06
1

A really easy way out would be to just fob this off onto .NET configuration and use the parameterless constructor which will grab the SMTP settings out of this app's configuration. This will let you inject appropriate values as required and probably the most SOLID thing you could do if you are staying static. See microsoft's man page on System.Net.Mail configuration for more info.

But since you want to learn here I would start by eliminating the whole static method for sending email approach -- it is definitely better than handling all of this by hand anytime you want to send an email. You would be much better served using an interface to provide a facade for sending email and then swapping in implementations as you see fit. So you'll want something like:

public interface IEmailService
{
    MessageSentResult SendMessage(MailMessage msg);
}

public class MessageSentResult
{
    public bool Successful { get; set; }
    public string ErrorCode { get; set; }
    public string ErrorMessage { get; set;}
    public MailMessage SentMessage { get; set; }
    public string FailedRecipients { get; set; }
}

Which might have an implementation that looks like:

    private readonly SmtpClient smtpClient;

    public SystemNetEmailService(SmtpClient smtpClient)
    {
        if (smtpClient == null) throw new ArgumentNullException("smtpClient");
        this.smtpClient = smtpClient;
    }

    public MessageSentResult SendMessage(MailMessage msg)
    {
        if (msg == null) throw new ArgumentNullException("msg");
        var res = new MessageSentResult()
                      {
                          SentMessage = msg,
                      };
        try
        {
            smtpClient.Send(msg);
            res.Successful = true;
            res.ErrorMessage = "No error.";
        }
        catch (SmtpFailedRecipientException mfex)
        {
            res.ErrorCode = mfex.StatusCode.ToString();
            res.ErrorMessage = "Some recipeints failed. See FailedRecipients for details.";
            res.FailedRecipients = mfex.FailedRecipient;
        }
        catch (SmtpException sex)
        {
            res.ErrorCode = sex.StatusCode.ToString();
            res.ErrorMessage = sex.Message;
            res.Successful = false;
        }
        return res;
    }

The main advantage here is testability and swappability. Testabiltiy inasmuch as you can easily build a fake implementation that doesn't send mail but could be used in your unit tests to check the message is correct. Swappability meaning you can do things like swap in a completely different implementation like backhauling your mail out over something like mailgun or mandrill because you realized delivery matters.

  • 1
    so where would I enter the SMTP host details ("my.smpthost.com")? – Prabhu Aug 15 '14 at 20:38
  • 1
    In configuration somewhere -- the class takes a dependency on the instantiated SmtpClient. Where that comes from it does not care. – Wyatt Barnett Aug 15 '14 at 20:53
  • 1
    @WyattBarnett: So, your answer answers everything except the question? It's a good answer, with good information otherwise. – Magus Aug 15 '14 at 21:03
  • @Magus -- see the 1st paragraph, I do answer the question. I expanded it a bit to make it more obvious. – Wyatt Barnett Aug 15 '14 at 21:28
  • But then, the SmtpClient needs to be configured. How would you pass in the SmtpClient Host configuration from the app.config or Web.config ? I've similar concern which I can't just pass in a new instance of Configuration, per say. – Will Marcouiller Jun 6 at 13:37

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.