10
\$\begingroup\$

Writing code to validate emails is a thing I don't like. It's usually pretty pointless as there's there is actually no right solution, so it leaves me with a certain sense of existential dread.

I've written some code that delegates most of the responsibility to System.Net.Mail.MailAddress. Unlike MailAddress, it plays nice with the NULL Address (this is where bounced emails come from) and with invalid addresses. It also behaves as an immutable value type, which is a personal preference of mine.

Is this code good enough that I can ignore email addresses for the rest of my life?

using System;
using System.Net.Mail;

public class EmailAddress
{
    private bool? _isValid = null;

    public string DisplayName { get; }
    public string Address { get; }

    public bool IsValid
    {
        get
        {
            if (!_isValid.HasValue)
            {
                _isValid = IsAddressValid(Address);
            }

            return _isValid.Value;
        }
    }

    public bool IsNullAddress
    {
        get { return string.IsNullOrEmpty(Address); }
    }

    public EmailAddress()
        : this(null, null)
    {
    }

    public EmailAddress(string emailAddress) : this(null, emailAddress)
    {
    }

    public EmailAddress(string name, string address)
    {
        DisplayName = !string.IsNullOrWhiteSpace(name) ? name : null;
        Address = !string.IsNullOrWhiteSpace(address) ? address : null;
    }


    public MailAddress ToMailAddress()
    {
        if (IsValid && !IsNullAddress)
            return new MailAddress(Address, DisplayName);

        throw new InvalidOperationException(
            "A MailAddress cannot be created from a invalid address or the null address." +
            " Call the IsValid and IsNullAddress properties first to avoid this exception."
        );
    }

    public static bool IsAddressValid(string mailAddress)
    {
        if (string.IsNullOrWhiteSpace(mailAddress))
        {
            return false;
        }

        try
        {
            var address = new MailAddress(mailAddress);

            // Check MailAddress didn't adjust/parse the value to create a valid MailAddress
            return string.Equals(address.Address, mailAddress, StringComparison.InvariantCultureIgnoreCase);
        }
        catch (Exception)
        {
            return false;
        }        
    }

    public override bool Equals(object obj)
    {
        if (ReferenceEquals(null, obj)) return false;
        if (ReferenceEquals(this, obj)) return true;
        if (obj.GetType() != this.GetType()) return false;
        return Equals((EmailAddress)obj);
    }

    protected bool Equals(EmailAddress other)
    {
        return string.Equals(DisplayName, other.DisplayName, StringComparison.InvariantCultureIgnoreCase) &&
            string.Equals(Address, other.Address, StringComparison.InvariantCultureIgnoreCase);
    }

    public override int GetHashCode()
    {
        unchecked
        {
            return ((DisplayName != null ? StringComparer.InvariantCultureIgnoreCase.GetHashCode(DisplayName) : 0) * 397) ^ 
                (Address != null ? StringComparer.InvariantCultureIgnoreCase.GetHashCode(Address) : 0);
        }
    }

    public override string ToString()
    {
        if (!string.IsNullOrEmpty(DisplayName))
        {
            return string.Format("{0} <{1}>",DisplayName, Address);
        }

        if (!IsNullAddress)
        {
            return Address;
        }

        return "NULL Address <>";
    }
}

And of course the unit tests:

using System;
using System.Collections.Generic;
using System.Linq;
using FluentAssertions;
using Microsoft.VisualStudio.TestTools.UnitTesting;

namespace ThisIsMyAwesomeNamespace
{
    [TestClass]
    public class EmailAddressTests
    {
        // https://en.wikipedia.org/wiki/Email_address#Valid_email_addresses

        private readonly List<string> _validEmails = new List<string>()
        {
            "prettyandsimple@example.com",
            "very.common@example.com",
            "disposable.style.email.with+symbol@example.com",
            "other.email-with-dash@example.com",
            "\"much.more unusual\"@example.com",
            "\"very.unusual.@.unusual.com\"@example.com",
            @"""very.(),:;<>[]\"".VERY.\""very@\\ \""very\"".unusual""@strange.example.com",
            "admin@mailserver1",
            "#!$%&'*+-/=?^_`{}|~@example.org",
            @"""()<>[]:,;@\\\""!#$%&'*+-/=?^_`{}| ~.a""@example.org",
            "\" \"@example.org",
            "example@localhost",
            "example@s.solutions",
            "user@com",
            "user@localserver",
            "user@[IPv6: 2001:db8::1]",
        };

        private readonly List<string> _invalidEmails = new List<string>()
        {
            "Abc.example.com",
            "A@b@c@example.com",
            @"a""b(c)d,e:f;g<h>i[j\k]l@example.com",
            "just\"not\"right@example.com",
            @"this is""not\allowed@example.com",
            @"this\ still\""not\\allowed@example.com",

            // While strictly not valid, MailAddress accepts these.
            //"john..doe@example.com",
            // "john.doe@example..com",

            // Mail Address actually accepts these because it parses the address from this strings
            // That doesn't make them valid email addresses, however
            "   leadingSpace@example.com",
            "trailingSpace@example.com     ",
            "displayname email@address.com",
            "<foo@bar.com>"
        };

        [TestMethod]
        public void ValidEmailsAreValid()
        {
            _validEmails.ForEach(emailStr =>
            {
                var emailAddress = new EmailAddress(emailStr);
                Assert.IsTrue(emailAddress.IsValid, string.Format("{0} should be a valid email", emailStr));
            });
        }

        [TestMethod]
        public void InvalidEmailsAreInvalid()
        {
            _invalidEmails.ForEach(emailStr =>
            {
                var emailAddress = new EmailAddress(emailStr);
                Assert.IsFalse(emailAddress.IsValid, string.Format("{0} should be an invalid email",emailStr));
            });
        }

        [TestMethod]
        public void NullAddressCanBeCreated()
        {
            var stringyNullAddress = new EmailAddress(string.Empty, string.Empty);
            var nullptryNullAddress = new EmailAddress(null, null);
            var emptyCtrNullAddress = new EmailAddress();

            Assert.IsTrue(stringyNullAddress.IsNullAddress);
            Assert.IsTrue(nullptryNullAddress.IsNullAddress);
            Assert.IsTrue(emptyCtrNullAddress.IsNullAddress);

            Assert.IsFalse(stringyNullAddress.IsValid);
            Assert.IsFalse(nullptryNullAddress.IsValid);
            Assert.IsFalse(emptyCtrNullAddress.IsValid);
        }

        [TestMethod]
        public void NullAddressCanBeTransformedIntoMailBoxAddress()
        {
            var nullAddress = new EmailAddress();

            var nullMailbox = nullAddress.ToMailboxAddress();

            Assert.IsTrue(nullMailbox.Address == string.Empty);
            Assert.IsTrue(nullMailbox.Name == string.Empty);

            // and matches the static property
            Assert.IsTrue(nullMailbox.Address == EmailAddress.NullMailboxAddress.Address);
            Assert.IsTrue(nullMailbox.Name == EmailAddress.NullMailboxAddress.Address);
        }

        [TestMethod]
        public void ValidAddressCanBeTransformedIntoMailBoxAddress()
        {
            var address = new EmailAddress("qaa", "foo@bar.com");
            var mailbox = address.ToMailboxAddress();

            Assert.AreEqual(address.Address, mailbox.Address);
            Assert.AreEqual(address.DisplayName, mailbox.Name);
        }

        [TestMethod]
        public void InvalidAddressCanBeTransformedIntoMailBoxAddress()
        {
            var address = new EmailAddress("qaa", "fdsfdsfdsf___");
            Assert.IsFalse(address.IsValid);
            var mailbox = address.ToMailboxAddress();

            Assert.AreEqual(address.Address, mailbox.Address);
            Assert.AreEqual(address.DisplayName, mailbox.Name);
        }

        [TestMethod]
        public void NullAddressCannotBeTransformedIntoMailAddress()
        {
            var nullAddress = new EmailAddress();

            Action createMailAddressFromNullAddress = () => nullAddress.ToMailAddress();

            createMailAddressFromNullAddress.ShouldThrow<InvalidOperationException>();
        }

        [TestMethod]
        public void ValidAddressCanBeTransformedIntoMailAddress()
        {
            var address = new EmailAddress("foo", _validEmails.First());
            var mailAddress = address.ToMailAddress();

            Assert.AreEqual(address.Address, mailAddress.Address);
            Assert.AreEqual(address.DisplayName, mailAddress.DisplayName);
        }

        [TestMethod]
        public void InvalidAddressCannotBeTransformedIntoMailAddress()
        {
            var address = new EmailAddress("foo", _invalidEmails.First());
            Assert.IsFalse(address.IsValid);
            Action createMailAddressFromInvalidAddress = () => address.ToMailAddress();

            createMailAddressFromInvalidAddress.ShouldThrow<InvalidOperationException>();
        }
    }
}

I just though I'd add this as well, although not central to the code I want reviewed. I've thought about what one does when MailAddress is missing (I'm looking at you Portable Class Libaries). 

    private static readonly TimeSpan RegexMatchTimeout = TimeSpan.FromMilliseconds(250);

    /// <summary>
    /// Portible version of http://msdn.microsoft.com/en-us/library/01escwtf%28v=vs.110%29.aspx
    /// </summary>
    public static bool IsAddressValid(string mailAddress)
    {
        if (string.IsNullOrWhiteSpace(mailAddress))
        {
            return false;
        }

        // Replace all unicode chars with ascii "A". Unicode is fine in emails, but not with this regex.
        var stringUnderTest = Regex.Replace(mailAddress, "[^\x0d\x0a\x20-\x7e\t]", "A");

        // Return true if stringUnderTest is in valid e-mail format. 
        try
        {
            return Regex.IsMatch(stringUnderTest,
                  @"^(?("")("".+?(?<!\\)""@)|(([0-9a-z]((\.(?!\.))|[-!#\$%&'\*\+/=\?\^`\{\}\|~\w])*)(?<=[0-9a-z])@))" +
                  @"(?(\[)(\[(\d{1,3}\.){3}\d{1,3}\])|(([0-9a-z][-\w]*[0-9a-z]*\.)+[a-z0-9][\-a-z0-9]{0,22}[a-z0-9]))$",
                  RegexOptions.IgnoreCase, RegexMatchTimeout);
        }
        catch (RegexMatchTimeoutException)
        {
            return false;
        }
    }

But it fails for addresses "admin@mailserver1", "#!$%&'*+-/=?^_`{}|~@example.org", "example@localhost", "user@com", "user@localserver" and "user@[IPv6: 2001:db8::1]". So, unless a regex expert wants to chip in, this is a problem I've not solved.

Improve this question
\$\endgroup\$
3
  • \$\begingroup\$ I agree, validating email addresses is one of the least productive and most frustrating things ever. +1 for what looks like a very reasonable solution :) \$\endgroup\$
    – Phrancis
    Apr 29, 2016 at 15:48
  • 1
    \$\begingroup\$ You could consider using the Lazy<T> class if your target allows it \$\endgroup\$
    – miniBill
    Apr 29, 2016 at 19:40
  • \$\begingroup\$ You can also actually check with an MX server if it responds for the specific email address. I also think most sophisticated java frameworks and APIs should have this function built in. \$\endgroup\$
    – Niklas Rosencrantz
    May 1, 2016 at 1:49

2 Answers 2

Reset to default
5
\$\begingroup\$ 
return Regex.IsMatch(stringUnderTest,
      @"^(?("")("".+?(?<!\\)""@)|(([0-9a-z]((\.(?!\.))|[-!#\$%&'\*\+/=\?\^`\{\}\|~\w])*)(?<=[0-9a-z])@))" +
      @"(?(\[)(\[(\d{1,3}\.){3}\d{1,3}\])|(([0-9a-z][-\w]*[0-9a-z]*\.)+[a-z0-9][\-a-z0-9]{0,22}[a-z0-9]))$",
      RegexOptions.IgnoreCase, RegexMatchTimeout);

I don't see why this needs to be a single regex. I think it would be better if you checked the required properties one by one using C# code. The resulting code would have many more characters, but would be much more readable and maintainable.

(My guess is that this approach would be slower than a compiled regex, but it seems you're not too worried about performance, since you're not using a compiled regex.)

public class EmailAddress
{
    public MailAddress ToMailAddress()

That's pretty confusing naming, the difference between EmailAddress and MailAddress is too small. I think you need a better name for your class, assuming the user is expected to use both classes (which this method implies).

public bool IsNullAddress
{
    get { return string.IsNullOrEmpty(Address); }
}

Since you seem to be using C# 6.0, consider expression-bodied property here:

public bool IsNullAddress => string.IsNullOrEmpty(Address);

protected bool Equals(EmailAddress other)

Why is this method protected? Instead, you should make it public and also implement IEquatable<EmailAddress>.

Also, your class doesn't seem to be designed for inheritance, so consider making it sealed and using private instead of protected.

Improve this answer
\$\endgroup\$
4
\$\begingroup\$

Instead of this:

public EmailAddress(string name, string address)
{
    DisplayName = !string.IsNullOrWhiteSpace(name) ? name : null;
    Address = !string.IsNullOrWhiteSpace(address) ? address : null;
}

Do this:

public EmailAddress(string name, string address)
{
    DisplayName = string.IsNullOrWhiteSpace(name) ? null : name;
    Address = string.IsNullOrWhiteSpace(address) ? null : address;
}
Improve this answer
\$\endgroup\$

Your Answer

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

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