9
\$\begingroup\$

I have the following code: (emailList type is List<string>)

//Add All emails
foreach (var mail in emailList)
{
    var isEmail = Regex.IsMatch(mail.Trim(), @"\A(?:[a-z0-9!#$%&'*+/=?^_`{|}~-]+(?:\.[a-z0-9!#$%&'*+/=?^_`{|}~-]+)*@(?:[a-z0-9](?:[a-z0-9-]*[a-z0-9])?\.)+[a-z0-9](?:[a-z0-9-]*[a-z0-9])?)\Z", RegexOptions.IgnoreCase);

    if (isEmail)
    {
        email.BccRecipients.Add(mail.Trim());
        sbInfoLog.Append($"Email: {mail.Trim()}\n");
    }
    else
    {
        sbAlertLog.Append($"Not valid email address: {mail}\n");
    }
}

I'm looking for another way to:

  1. To validate the email address
  2. To Iterate over the list (Maybe with ForEach etc..)

Any suggestions?

\$\endgroup\$
  • 3
    \$\begingroup\$ ex-parrot.com/~pdw/Mail-RFC822-Address.html - tl;dr the only way to properly validate an email address is to send it an email. \$\endgroup\$ – MikeTheLiar Feb 27 '17 at 22:07
  • 1
    \$\begingroup\$ Email addresses have a lot of gotchas, so trying to validate them with a program is troublesome, and trying to validate them with a regex is a nightmare. \$\endgroup\$ – user97178 Feb 27 '17 at 23:16
14
\$\begingroup\$

This is a method I've used without any problems so far.

bool IsValidEmail(string email)
{
    try 
    {
        var addr = new System.Net.Mail.MailAddress(email);
        return true;
    }
    catch 
    {
        return false;
    }
}

To check the validity and handle each case you could do this.

var emailChecklist = emails.Select(x => 
    new { IsValid = IsValidEmail(x.Trim()), Address = x });

foreach(var email in emailChecklist)
{
    if (email.IsValid)
    {
        email.BccRecipients.Add(email.Address.Trim());
        sbInfoLog.Append($"Email: {email.Address.Trim()}\n");
    }
    else
    {
        sbAlertLog.Append($"Not valid email address: {email.Address}\n");
    }
}

I left the trim in the loop because I noticed that you didn't trim on the invalid email log. I assumed that you were trying to preserve the original value.

\$\endgroup\$
  • 6
    \$\begingroup\$ +1 better to use the MailAddress constructor's validation than to try (and usually fail) at validating addresses via regex \$\endgroup\$ – Dan Lyons Feb 27 '17 at 19:15
  • 4
    \$\begingroup\$ Even better: Use semantically meaningful types throughout the program. Then you can never accidentally confuse an arbitrary string with an e-mail address. It also helps to search the web for email validation regex. Once I did that, I knew that I never want to get into that business. \$\endgroup\$ – 5gon12eder Feb 27 '17 at 21:23
  • 3
    \$\begingroup\$ +1 for using LINQ and anonymous types, and definitely +1 for using .NET's built-in email class to do the validation! That snippet should be in every self-respecting .NET dev's toolbox. \$\endgroup\$ – MarioDS Feb 27 '17 at 22:19
  • 3
    \$\begingroup\$ For anyone curious (as I was), MailAddress uses a much more RFC-compliant parser instead of a simple (or complicated) RegEx. Unfortunately this parser (MailAddressParser) is in an internal class so the only way to access it is via the MailAddress constructor. \$\endgroup\$ – Nick Feb 28 '17 at 0:34
  • 2
    \$\begingroup\$ Correction on my earlier statement: 'it [the MailAddress method] also fails 106 out of the 168 tests': it "only" fails 38 cases depending on how 'lax' you want to interpret the RFC's / allow deprecated things etc. \$\endgroup\$ – RobIII Feb 28 '17 at 16:03
3
\$\begingroup\$

You can use the ToLookup extension to group the emails by validation result. To access the collection you use either true or false. With this approach you can split the invalid/valid email handling.

You also should put the very long pattern in a constant.

var emails = new[] { "..", ".." };

const string EmailPattern = "..";

var validEmails = emails
    .Select(x => x.Trim())
    .ToLookup(x => Regex.IsMatch(x, EmailPattern, RegexOptions.IgnoreCase));

foreach (var email in validEmails[true])
{
    // ...
}

// do something else with validEmails[false]
\$\endgroup\$
3
\$\begingroup\$

I forget the syntax but regex is run time processed so define the regex outside the loop and call it inside the loop. If you notice the compiler will not pick up regex syntax errors. You don't get them until run time. It is significant. When I tested creating the Regex run time took longer than processing.

I would just trim up front

mail = mail.Trim();

And I would break out the pattern in a separate string so if you change it you are touching less.

public static void ValidateEmail()
{
    List<string> emailList = new List<string>() { "emailList@se.com", "Not Valid Email"  };
    List<string> emailListGood = new List<string>();
    StringBuilder sbAlertLog = new StringBuilder();
    string pattern = @"\A(?:[a-z0-9!#$%&'*+/=?^_`{|}~-]+(?:\.[a-z0-9!#$%&'*+/=?^_`{|}~-]+)*@(?:[a-z0-9](?:[a-z0-9-]*[a-z0-9])?\.)+[a-z0-9](?:[a-z0-9-]*[a-z0-9])?)\Z";
    Regex regex = new Regex(pattern, RegexOptions.IgnoreCase);
    string mailTrim;
    foreach (var mail in emailList)
    {
        mailTrim = mail.Trim();
        if (regex.IsMatch(mailTrim))
        {
            emailListGood.Add(mailTrim);
        }
        else
        {
            sbAlertLog.Append($"Not valid email address: {mailTrim}\n");
        }
    }
}
\$\endgroup\$

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.