C# – Passing in config values to a method using SOLID principles

cnetobject-orientedobject-oriented-designsolid

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.)

Best Answer

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.

Related Topic