HiveBrain v1.2.0
Get Started
← Back to all entries
snippetcsharpMinor

Implement mail sending system elegantly

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
implementsystemsendingmailelegantly

Problem

In my project I need to send emails. Emails will have different credentials and senders. I'm thinking about "abstract factory", but will it be good solution?

public abstract class Mail
    {
        public string Body { get; protected set; }
        public string Subject { get; private set; }
        public string EmailTo { get; private set; }

        protected Mail(string subject, string emailTo)
        {
            this.Subject = subject;
            this.EmailTo = emailTo;
        }

        protected abstract void BuildBody();
    }
public class AccountMail : Mail
    {
        private string _firstName;
        private string _lastName;
        private string _email;
        private string _siteDomain;
        public AccountMail(string subject, string emailTo, string firstName, string lastName, string siteDomain, string email): base(subject, emailTo)
        {
            _firstName = firstName;
            _lastName = lastName;
            _email = email;
            _siteDomain = siteDomain;
            BuildBody();            
        }

        protected override sealed void BuildBody()
        {   
             StringBuilder bodyBuilder = new StringBuilder();
             base.Body = bodyBuilder.ToString();
        }


That is abstract Mail and concrete Mail. Do I really need abstract MailSender and concrete MailSender for each email? This is quick solution, because I needed this functionality right now. It's working like this:

```
public abstract class MailSender
{
public abstract void SendMail(string subject, string emailTo, string firstName, string lastName);
}
public class AccountMailSender : MailSender
{
public override void SendMail(string subject, string emailTo, string firstName, string lastName)
{
AccountMail mail = new AccountMail(subject, emailTo, firstName, lastName, "");
Task.Factory.StartNew(() =>
{
MailMessage mailMessage = new M

Solution

Abstract base classes are only useful if you're inheriting functionality. So, what you've done with Mail is good, because all of your inheritors get their properties "for free". In contrast, there's no sense in your MailSender class. Inheritors get no functionality, but are forced to adhere to the contract you've decided on. That's what Interfaces are for.

What I actually don't like is that it's only possible to send an email to exactly one person with your classes. Very soon, you'll wish that you could send an email to a bunch of people at once. EmailTo should mimic the class that you're wrapping up and return a collection.

You're also missing any concept of From in your Mail class. I would expect an email message to know who it's coming from.

Context

StackExchange Code Review Q#116641, answer score: 4

Revisions (0)

No revisions yet.