Bring phone numbers in consistent format

Question

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!

Answer 1

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.

Answer 2

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");

Answer 3

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).

Answer 4

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