Please critique and criticize...I could use some advice

[Jason soby]

In our DB, we store all phone #s as Varchars, so when they come out, they are
strings instead of longs. Because of this, we can't just use a String.Format
on the field itself. Also, the db also stores any other non numerics such as
parens or dashes. So, if i want to format a phone number, i need to remove
all the non-numerics, convert it to a long, run String.Format on it, and then
return it as a formatted string.

Here is my code. Is this as optimized as it should be?

public static string FormatPhoneNumber(string phoneNumber)
{
string tempPhoneNumber;
long phoneNumberAsNumeric;

tempPhoneNumber = Regex.Replace(phoneNumber, @"\D",
String.Empty); //Remove all non-numerics
if (tempPhoneNumber.Length == 10) //We do NOT want to modify
international numbers
{
phoneNumberAsNumeric = Int64.Parse(tempPhoneNumber);
//Convert to a numeric since String.Format only works on non-strings
phoneNumber = String.Format("{0###) ###-####}",
phoneNumberAsNumeric); //Format to use the standard phone number formatting
}

return phoneNumber;
}

Thanks,
Jason

 

[Jeff Johnson]

if (tempPhoneNumber.Length == 10) //We do NOT want to modify
international numbers
{
phoneNumberAsNumeric = Int64.Parse(tempPhoneNumber);
//Convert to a numeric since String.Format only works on non-strings
phoneNumber = String.Format("{0###) ###-####}",
phoneNumberAsNumeric); //Format to use the standard phone number
formatting
}

My gut feeling is that the conversion to a number and a second conversion
with String.Format() is probably not efficient. Again, gut feeling with no
testing/profiling whatsoever.

Since you're limiting the formatting ONLY to strings that have 10 digits,
why not just substring?

phoneNumber = "(" + tempPhoneNumber.Substring(0, 3) + ") " +
tempPhoneNumber.Substring(3, 3) + "-" + tempPhoneNumber.Substring(6, 4);

 

[Peter Morris]

This app will only ever be used in the USA?

 

[Michael B. Trausch]

In our DB, we store all phone #s as Varchars, so when they come out,
they are strings instead of longs. Because of this, we can't just use
a String.Format on the field itself. Also, the db also stores any
other non numerics such as parens or dashes. So, if i want to format
a phone number, i need to remove all the non-numerics, convert it to
a long, run String.Format on it, and then return it as a formatted
string.

It is a bit restrictive.

You should be storing numbers (even if you only store US/CAN numbers)
as three strings: a country code, a local number, and any extension or
additional information that may be necessary when using the phone
number.

Any additional information that you get from the phone number (area
code, for example) is actually the "local number" portion. So, given:

14045551212

You have:

1 - the country code for the US (and Canada)
4045551212 - the local number, which (because of the country code) we
can deduce is:

404 - area code
555 - exchange
1212 - "extension"

Assuming that you wanted to store this with no extension, you'd have
the three strings:

1
4045551212
""

Assuming that you wanted to store it, and indicate that the person the
record is attached to is found at extension 1234 (either by asking at a
switchboard or dialing into an IVR), then you'd have the three strings:

1
4045551212
1234

HTH,
Mike

 

[tomilay]

In our DB, we store all phone #s as Varchars, so when they come out, theyare
strings instead of longs. Because of this, we can't just use a String.Format
on the field itself. Also, the db also stores any other non numerics suchas
parens or dashes. So, if i want to format a phone number, i need to remove
all the non-numerics, convert it to a long, run String.Format on it, and then
return it as a formatted string.

Here is my code. Is this as optimized as it should be?

        public static string FormatPhoneNumber(string phoneNumber)
        {
            string tempPhoneNumber;
            long phoneNumberAsNumeric;

            tempPhoneNumber = Regex.Replace(phoneNumber, @"\D",
String.Empty); //Remove all non-numerics
            if (tempPhoneNumber.Length == 10) //We do NOTwant to modify
international numbers
            {
                phoneNumberAsNumeric = Int64.Parse(tempPhoneNumber);
//Convert to a numeric since String.Format only works on non-strings
                phoneNumber = String.Format("{0###) ###-####}",
phoneNumberAsNumeric); //Format to use the standard phone number formatting
            }

            return phoneNumber;
        }

Thanks,
Jason

By first removing the non-numerics aren't you in effect modifying the
international numbers? Otherwise the Int64.Parse functionality is
probably unnecessary since you are guaranteed at that point in the
code to have a ten digit string.

So your code should probably look as below:

public static string FormatPhoneNumber(string phoneNumber)
{
string tempPhoneNumber;

tempPhoneNumber = Regex.Replace(phoneNumber, @"\D",
String.Empty); //Remove all non-numerics
if (tempPhoneNumber.Length == 10) //We do NOT want to
modify
international numbers
{
phoneNumber = String.Format("{0###) ###-####}",
tempPhoneNumber); //Format to use the standard phone number
formatting
}

return phoneNumber;
}

I could be wrong since my .NET dev environment is currently down and
am unable to confirm that this works.

 

[Cor Ligthert[MVP]]

Why is Internationaly non USA for me USA is Internationally?

I have read Jason's message more times, but seen any word about that.

Developing is not a matter of assuming

jmo

Cor

(+31)

 

[Göran Andersson]

In our DB, we store all phone #s as Varchars, so when they come out, they are
strings instead of longs. Because of this, we can't just use a String.Format
on the field itself. Also, the db also stores any other non numerics such as
parens or dashes. So, if i want to format a phone number, i need to remove
all the non-numerics, convert it to a long, run String.Format on it, and then
return it as a formatted string.

Here is my code. Is this as optimized as it should be?

public static string FormatPhoneNumber(string phoneNumber)
{
string tempPhoneNumber;
long phoneNumberAsNumeric;

tempPhoneNumber = Regex.Replace(phoneNumber, @"\D",
String.Empty); //Remove all non-numerics
if (tempPhoneNumber.Length == 10) //We do NOT want to modify
international numbers
{
phoneNumberAsNumeric = Int64.Parse(tempPhoneNumber);
//Convert to a numeric since String.Format only works on non-strings
phoneNumber = String.Format("{0###) ###-####}",
phoneNumberAsNumeric); //Format to use the standard phone number formatting
}

return phoneNumber;
}

Thanks,
Jason

There's a one-liner for everything.

return Regex.Replace(phoneNumber,
@"^\D*(\d)\D*(\d)\D*(\d)\D*(\d)\D*(\d)\D*(\d)\D*(\d)\D*(\d)\D*(\d)\D*(\d)\D*$",
"($1$2$3) $4$5$6-$7$8$9$10");

 

[Paul]

You will need to load a different regex depending on country. I know for a
fact that CLI's can be between 9/10 and 12 in the UK.

In our DB, we store all phone #s as Varchars, so when they come out, they
are
strings instead of longs. Because of this, we can't just use a
String.Format
on the field itself. Also, the db also stores any other non numerics such
as
parens or dashes. So, if i want to format a phone number, i need to remove
all the non-numerics, convert it to a long, run String.Format on it, and
then
return it as a formatted string.

Here is my code. Is this as optimized as it should be?

public static string FormatPhoneNumber(string phoneNumber)
{
string tempPhoneNumber;
long phoneNumberAsNumeric;

tempPhoneNumber = Regex.Replace(phoneNumber, @"\D",
String.Empty); //Remove all non-numerics
if (tempPhoneNumber.Length == 10) //We do NOT want to modify
international numbers
{
phoneNumberAsNumeric = Int64.Parse(tempPhoneNumber);
//Convert to a numeric since String.Format only works on non-strings
phoneNumber = String.Format("{0###) ###-####}",
phoneNumberAsNumeric); //Format to use the standard phone number
formatting
}

return phoneNumber;
}

Thanks,
Jason

By first removing the non-numerics aren't you in effect modifying the
international numbers? Otherwise the Int64.Parse functionality is
probably unnecessary since you are guaranteed at that point in the
code to have a ten digit string.

So your code should probably look as below:

public static string FormatPhoneNumber(string phoneNumber)
{
string tempPhoneNumber;

tempPhoneNumber = Regex.Replace(phoneNumber, @"\D",
String.Empty); //Remove all non-numerics
if (tempPhoneNumber.Length == 10) //We do NOT want to
modify
international numbers
{
phoneNumber = String.Format("{0###) ###-####}",
tempPhoneNumber); //Format to use the standard phone number
formatting
}

return phoneNumber;
}

I could be wrong since my .NET dev environment is currently down and
am unable to confirm that this works.