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)

\$\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

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