I'm using this code to bring my phone numbers in a consistent format.
Desired:
+(country code)phone number
Possible patterns:
01721234567
-> change to desired pattern
00491234567
-> change to desired pattern
+4912345678
-> do nothing, already desired pattern
This is what I use:
String number = allContactNumbers.get(i).get(j);
number = number.replaceAll("[^+0-9]", ""); // All weird characters such as /, -, ...
String country_code = getResources().getString(R.string.countrycode_de);
if (number.substring(0, 1).compareTo("0") == 0 && number.substring(1, 2).compareTo("0") != 0) {
number = "+" + country_code + number.substring(1); // e.g. 0172 12 34 567 -> + (country_code) 172 12 34 567
}
number = number.replaceAll("^[0]{1,4}", "+"); // e.g. 004912345678 -> +4912345678
For some reason I'm not happy with it though. I hope there is somebody to tell me, whether my code is written properly!
4 Answers 4
There are two aspects to this, the general design, and the implementation.
Implementation
The entire things should be extracted as a function. The first line of code:
String number = allContactNumbers.get(i).get(j);
indicates that this code is being run inside a loop (i,j). You need to extract it to be:
String number = normalizePhoneNumber(allContactNumbers.get(i).get(j));
and the function would look something like:
public String normalizePhoneNumber(String number) {
......
return normalized;
}
Right, about that function, putting your code in it ends up with:
public String normalizePhoneNumber(String number) {
number = number.replaceAll("[^+0-9]", ""); // All weird characters such as /, -, ...
String country_code = getResources().getString(R.string.countrycode_de);
if (number.substring(0, 1).compareTo("0") == 0 && number.substring(1, 2).compareTo("0") != 0) {
number = "+" + country_code + number.substring(1); // e.g. 0172 12 34 567 -> + (country_code) 172 12 34 567
}
number = number.replaceAll("^[0]{1,4}", "+"); // e.g. 004912345678 -> +4912345678
return number;
}
Design
OK, about the design.... I believe this is a problem. Handling phone numbers is much more complicated than what you have.... it is really a challenging problem.
It is easy enough to strip off the junk, but it gets hard really fast. For example, this is the the Queen of England's land-line number (not kidding):
Public Information Officer Buckingham Palace London SW1A 1AA Tel (during 9am - 5pm (GMT) Monday to Friday): (+44) (0)20 7930 4832. Please note, calls to this number may be recorded.
As a Canadian, the international dialing code for this would be:
01144207934832
How will your script translate that in to:
+442079305832
I think you need to revise your plan for this problem, it is more complicated than you realize.
-
\$\begingroup\$ I dont see why you have to dial
01144207934832
from canada instead of+442079305832
. If this number: ` (+44) (0)20 7930 4832` is provided, my script would first make it:+4402079304832
then in the second step it would erase the plus, delete all zeroes in the beginning (there are none in this example) then add the + again, this would lead to+4402079304832
, which is wrong, since there is a 0 after the country code, BUT nobody would have it in his adressbook like this. and if, its their fault. \$\endgroup\$Philip– Philip2014年04月03日 02:17:47 +00:00Commented Apr 3, 2014 at 2:17 -
1\$\begingroup\$ The 011 is the international direct dial (IDD) number that must be dialed before the country code in Canada and the US. In Germany the IDD is 0 and the UK the IDD is 00. \$\endgroup\$Edward– Edward2014年04月03日 02:37:18 +00:00Commented Apr 3, 2014 at 2:37
I don't have a handy way to check this code right now, but I believe it could be done like this:
// only keep digits
number = number.replaceAll("[^0-9]", "");
// trim leading zeroes
number = "+" + number.replaceFirst("^0*(.*)","1ドル");
-
\$\begingroup\$ but in this example you dont check whether country code is given or not \$\endgroup\$Philip– Philip2014年04月03日 02:18:45 +00:00Commented Apr 3, 2014 at 2:18
-
\$\begingroup\$ That's correct. Reducing a phone number with embedded spaces and parentheses and possibly leading zeroes is relatively simple. Picking out the country code is, as the other answer notes, much more complex. For example +1 is actually not a country code but a signifier of the North American numbering plan. Further, the US and Canada both use +1 with no other signifier. And it's not even tidy geographically. A number beginning with +1 809 is the Dominican Republic but Haiti (on the other end of the same island!) is +509. There's no simple way to untangle it. \$\endgroup\$Edward– Edward2014年04月03日 02:29:56 +00:00Commented Apr 3, 2014 at 2:29
-
1\$\begingroup\$ @Edward: "For example +1 is actually not a country code but a signifier of the North American numbering plan." -- That's a distinction without a difference. From an outsider's perspective it's simply a country code that corresponds to more than one country. It is isomorphic to a country code of "1" with a national dialing prefix of "1" being ignored. The fact that multiple countries fall within the code is irrelevant to almost all purposes you might have for this kind of algorithm. \$\endgroup\$Jules– Jules2014年04月03日 04:40:01 +00:00Commented Apr 3, 2014 at 4:40
-
\$\begingroup\$ @Jules: Many people, even in Europe, consider Canada and the US to be different countries. :) \$\endgroup\$Edward– Edward2014年04月03日 11:22:40 +00:00Commented Apr 3, 2014 at 11:22
The biggest problem I have is that your code assumes a particular pattern for national numbers and international numbers that really ought to be end-user configurable. You should have preferences for international prefix (probably defaulting to "00"), for local national code (which you are currently retrieving from a resource, which should instead be used to set the default value for the preference) and for local number national prefix (which you currently have hardwired to "0"). Then, assuming you have a method to retrieve a preference with the signature getPreferenceString(String preferenceName, String defaultValue)
, the algorithm would look something like:
String originalNumber = (whatever code you need to get the original number)
String internationalPrefix = getPreferenceString(PREF_INTERNATIONAL_PREFIX, "00");
String defaultCountryCode = getPreferenceString(PREF_DEFAULT_COUNTRY_CODE,
getResources().getString(R.string.countrycode_de));
String defaultCountryNationalPrefix = getPreferenceString(PREF_NATIONAL_PREFIX, "0");
// strip any non-significant characters
String number = originalNumber.replaceAll("[^0-9+]", "");
// check for prefixes
if (number.startsWith ("+")) // already in desired format
return number;
if (number.startsWith(internationalPrefix))
return number.replace(internationalPrefix, "+");
else if (number.startsWith(defaultCountryNationalPrefix))
return number.replace(defaultCountryNationalPrefix, "+" + defaultCountryCode);
else
throw new IllegalArgumentException ("Number " + originalNumber + " does not have either an international or a national prefix");
Note that this code can throw an exception if the phone number does not conform to expectations, e.g. if it lacks either an international or national prefix. In such a case, either the preferences are set incorrectly, or the phone has local numbers in its database, which the system I've described doesn't have enough information to translate. Such numbers wouldn't be usable on a mobile phone, as mobile networks do not typically support dialing local numbers, but I don't know that there aren't desk phones with Android installed on them somewhere...
As a side comment, you're doing your string comparisons in an odd way. You should use string.equals("expected value")
not string.compareTo("expected value") == 0
to check for equality (and !string.equals(...)
rather than ... != 0
for inequality) because (1) it's easier to read and (2) it's slightly more efficient at run time.
Also, there's a method startsWith(String)
. Use it, rather than extracting a substring and comparing the result, which is error-prone (if you get the length of the substring wrong the comparison will always fail, or if the string you're comparing is shorter than the prefix you're checking for an IndexOutOfBoundsException
will be thrown).
If you are going to use regular expressions, use them effectively so that you don't have to resort to low-level string manipulation. In particular, make use of capturing parentheses to figure out what was found during matching.
public class TelephoneNumberCanonicalizer {
private static final Pattern EUROPEAN_DIALING_PLAN = Pattern.compile("^\\+|(00)|(0)");
private final String countryCode;
public TelephoneNumberCanonicalizer(String countryCode) {
this.countryCode = countryCode;
}
public String canonicalize(String number) {
// Remove all weird characters such as /, -, ...
number = number.replaceAll("[^+0-9]", "");
Matcher match = EUROPEAN_DIALING_PLAN.matcher(number);
if (!match.find()) {
throw new IllegalArgumentException(number);
} else if (match.group(1) != null) { // Starts with "00"
return match.replaceFirst("+");
} else if (match.group(2) != null) { // Starts with "0"
return match.replaceFirst("+" + this.countryCode);
} else { // Starts with "+"
return number;
}
}
}
Test case
public static void main(String[] args) {
String[] testCases = new String[] {
"01721234567",
"00491234567",
"+4912345678"
};
TelephoneNumberCanonicalizer german = new TelephoneNumberCanonicalizer("49");
for (String testCase : testCases) {
System.out.printf("%s -> %s\n", testCase, german.canonicalize(testCase));
}
}
Output
01721234567 -> +491721234567
00491234567 -> +491234567
+4912345678 -> +4912345678
+
and all leading zeroes as step 1, and then just add the leading+
at the end? No country code begins with 0. \$\endgroup\$