5
\$\begingroup\$

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?

\$\endgroup\$
6
\$\begingroup\$

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 explicit 1 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)

| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ You may want to add ^ at the start and $ at the end of the pattern, because the OP asked to match the entire input string. \$\endgroup\$ – miasbeck Jan 15 '15 at 10:06
  • \$\begingroup\$ @miasbeck good point - I'll make the edit. \$\endgroup\$ – Dan Lyons Jan 15 '15 at 18:28

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy