patterncsharpMinor
Can I stop validating email addresses now please?
Viewed 0 times
canemailpleasevalidatingstopnowaddresses
Problem
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;
}
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;
}
Solution
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.Also, your class doesn't seem to be designed for inheritance, so consider making it
sealed and using private instead of protected.Code Snippets
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);public class EmailAddress
{
public MailAddress ToMailAddress()public bool IsNullAddress
{
get { return string.IsNullOrEmpty(Address); }
}public bool IsNullAddress => string.IsNullOrEmpty(Address);protected bool Equals(EmailAddress other)Context
StackExchange Code Review Q#127065, answer score: 5
Revisions (0)
No revisions yet.