Need a better way to format a phone number in C
I have a character array that contains a phone number of the form: "(xxx) xxx-xxxx xxxx" and need to convert it to something like the form: "xxx-xxx-xxxx" where I just truncate the extension. My initial pass in the function looks like this:
static void formatPhoneNum( char *phoneNum ) {
unsigned int i;
int numNumbers = 0;
/* Change the closing parenthesis to a dash and truncate at 12 chars. */
for ( i = 0; i < strlen( phoneNum ); i++ ) {
if ( phoneNum[i] == ')' ) {
phoneNum[i] = '-';
}
else if ( i == 13 ) {
phoneNum[i] = '\0';
break;
}
else if ( isdigit( phoneNum[i] ) ) {
numNumbers++;
}
}
/* If the phone number is empty or not a full phone number,
* i.e. just parentheses and dashes, or not 10 numbers
* format it as an emtpy string. */
if ( numNumbers != 10 ) {
strcpy( phoneNum, "" );
}
else {
/* Remove the first parenthesis. */
strcpy( phoneNum, phoneNum + 1 );
}
}
It seems to me that I want me to remove the leading pair, but I cannot just increment the pointer in the function as the pointer of the calling version will not update. I also feel that I can be "smarter" in general in the whole function.
Any ideas / pointers?
source to share
Since you stated that your input is guaranteed to be in the correct format, how about the following:
static void formatPhoneNum( char *phoneNum )
{
memmove(phoneNum, phoneNum + 1, 12);
phoneNum[3] = '-';
phoneNum[12] = 0;
}
memmove () is guaranteed to work with overlapping buffers
source to share
As Paul said, you cannot bind a string to yourself. I am declaring a new variable for clarity, although my approach does not use strcpy - with care, you can reuse the original variable. Anyway, if your input is always of the form (xxx) xxx-xxxx xxxx and your output is always xxx-xxx-xxxx, why not just do:
char newPhone[14];
newPhone[0] = phoneNum[1];
newPhone[1] = phoneNum[2];
newPhone[2] = phoneNum[3];
newPhone[3] = '-';
newPhone[4] = phoneNum[6];
newPhone[5] = phoneNum[7];
newPhone[6] = phoneNum[8];
newPhone[7] = '-';
newPhone[8] = phoneNum[10];
newPhone[9] = phoneNum[11];
newPhone[10] = phoneNum[12];
newPhone[11] = phoneNum[13];
newPhone[12] = '\0';
Brute force? Sure, but - if your ins and outs are always what you claim, they should work efficiently.
source to share
Well, I guess I'm too slow. Nothing smart about memmove () in this regard, but it shows how you can have a loop and still do all these comparisons from the inside:
char *formatPhoneNum(char *buffer) {
int index = 0;
for( index = 0; index < 12; ++index ) {
buffer[index] = buffer[index + 1];
}
buffer[3] = '-';
buffer[12] = '\0';
return buffer;
}
You might find it helpful if you return the beginning of the line you are modifying, rather than just void, to simplify your command chain. For example,
printf("%s\n", formatPhoneNum(buffer));
source to share
To begin with, this is not true:
strcpy( phoneNum, phoneNum + 1 );
as the ISO C standard says strcpy
:
If copying occurs between overlapping objects, the behavior is undefined.
"objects" here are source and target arrays char
. MSDN agrees with this, by the way, so it won't work as expected in at least one popular real-world implementation.
It looks like a simpler approach would be for the function to return the new "correct" pointer value (in the same buffer), so he can set it to 1 to truncate '('
.
Your check, which just counts the numbers, allows formatting, for example, "1-234567890"
or "1234567890-"
or even "12345foobar4567890"
- this may or may not be an issue, depending on the requirements.
source to share
Whenever possible (and does not degrade performance), I prefer to pass data to functions as const. In your case, I see no reason not to do this, so I declare your function as
static void formatPhoneNum(char *dst, const char *src);
or even, returning the length of the new number:
static int formatPhoneNum(char *dst, const char *src);
Then just copy the numbers from src
to dst
by pasting the dashes where you want . The caller is responsible for providing space in dst
and checks the return value: if 12 (hyphens included), everything is fine; otherwise, an error occurred.
You can return a negative number to indicate possible errors. For example: -1 indicates that it is src
not long enough; -2 indicates bad format for src
, etc.
Document all return values!
ABOUT! And don't forget that NUL completes dst
!
source to share
If you are allowed to change the API, you can either accept char ** or return char * and improve the time complexity:
static void formatPhoneNum(char **phoneNum) {
(*phoneNum)[4] = '-';
(*phoneNum)[13] = '\0';
(*phoneNum)++;
}
On the other hand
static char *formatPhoneNum(char *phoneNum) {
phoneNum[4] = '-';
phoneNum[13] = '\0';
return phoneNum + 1;
}
The advantage is that it will take constant time.
source to share