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

7
  • 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
    Commented Aug 15, 2014 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
    Commented Aug 15, 2014 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
    Commented Aug 15, 2014 at 20:39
  • Well in that case, the answer is D: They should be passed in.
    – Magus
    Commented Aug 15, 2014 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
    Commented Aug 15, 2014 at 21:06

1 Answer 1

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.

5
  • 1
    so where would I enter the SMTP host details ("my.smpthost.com")?
    – Prabhu
    Commented Aug 15, 2014 at 20:38
  • 1
    In configuration somewhere -- the class takes a dependency on the instantiated SmtpClient. Where that comes from it does not care. Commented Aug 15, 2014 at 20:53
  • 2
    @WyattBarnett: So, your answer answers everything except the question? It's a good answer, with good information otherwise.
    – Magus
    Commented Aug 15, 2014 at 21:03
  • @Magus -- see the 1st paragraph, I do answer the question. I expanded it a bit to make it more obvious. Commented Aug 15, 2014 at 21:28
  • 1
    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. Commented Jun 6, 2019 at 13:37

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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