Problem
I have written my first regex:
static void Main(string[] args)
{
string phoneNumber= Console.ReadLine();
Regex pattern = new Regex("(1-)?\p{N}{3}-\p{N}{3}-\p{N}{4}\b");
MatchCollection collection = pattern.Matches(phoneNumber);
Console.WriteLine(phoneNumber + " is " + (collection.Count == 1 && collection[0].ToString() == phoneNumber ? "" : "not ") + "a valid US number.");
}
It needs to match the entire input string, and it can work for any US phone number with or without country code “1-“. It can most likely be improved; any suggestions?
Solution
Use of Regex Objects
If your intent is merely to validate that the input is a phone number or not, rather than trying to capture anything, you can use the static Regex.IsMatch function:
var isPhoneNumber = Regex.IsMatch(phoneNumber, "(1-)?\p{N}{3}-\p{N}{3}-\p{N}{4}\b");
That saves having to deal with creating a new object and deal with a MatchCollection
Regex Pattern
There are a few things I would change:
- There is a simple character class for matching digits 0-9:
d
.p{N}
may be more appropriate if you need to match the entire Unicode number group rather than simple digits, but then I would not expect to see the explicit1
at the start. b
matches backspace characters, which seems a little odd in a phone number. I would omit it. If your phone number strings happen to have backspace characters at the end, it will still match, but I would not require it be there.- Given the backslashes necessary for character escapes, I would prefix your string with the
@
symbol for readability
Also, as @miasbeck points out, if you want to match the entire input string, you need to prefix the pattern with ^
and suffix it with $
. They indicate that the match must start at the beginning and end of the string, respectively.
The end result would be something like the following:
@"^(1-)?d{3}-d{3}-d{4}$"
Additionally, since there is no requirement that an area code is used for local numbers, so you may want to make it optional as well:
@"^((1-)?d{3}-)?d{3}-d{4}$"
(note: The MSDN page on Regular Expression Language is always a good reference to check on when coming up with regex patterns in .NET code.)
String Formatting
Rather than concatenating the strings together, I would use a format string:
const string FMT = "{0} is {1} a valid US number.";
Additionally, instead of using ""
, I always tend towards string.Empty
, as it is much more explicit about intent. ""
could easily be a mistake, but string.Empty
is clear that you really do want an empty string.
You can then output your result as follows:
Console.WriteLine(FMT, phoneNumber, (isPhoneNumber ? string.Empty : "not "));
Bringing it Together
Taking all the suggestions together, you get the following:
static void Main(string[] args)
{
const string FMT = "{0} is {1} a valid US number.";
var phoneNumber= Console.ReadLine();
var isPhoneNumber = Regex.IsMatch(phoneNumber, @"^((1-)?d{3}-)?d{3}-d{4}$");
Console.WriteLine(FMT, phoneNumber, (isPhoneNumber ? string.Empty : "not "));
}
(note: I switch back and forth between C# and F# , so I use var
more often than most – feel free to keep the explicit type declarations if you really want)