Fix the implementation of phone_number_compare in accordance with the tests in PhoneNumberUtils.java.
Due to the Thailand's special case, the code became a little nastier than before, but I believe it is inevitable...
diff --git a/android/Android.mk b/android/Android.mk
index 44d77b6..41675f2 100644
--- a/android/Android.mk
+++ b/android/Android.mk
@@ -40,9 +40,13 @@
# external/sqlite/android/PhoneNumberUtilsTest.cpp
# > ./a.out
#
-# Note: tests related to PHONE_NUMBERS_EQUAL also exists in AndroidTests in
-# java space. Add tests if you modify this.
-
+# Note: This "test" is not recognized as a formal test. This is just for enabling developers
+# to easily check what they modified works well or not.
+# The formal test for phone_number_compare() is in DataBaseGeneralTest.java
+# (as of 2009-08-02), in which phone_number_compare() is tested via sqlite's custom
+# function "PHONE_NUMBER_COMPARE".
+# Please add tests if you modify the implementation of PhoneNumberUtils.cpp and add
+# test cases in PhoneNumberUtilsTest.cpp.
include $(CLEAR_VARS)
LOCAL_MODULE:= libsqlite3_phone_number_utils_test
diff --git a/android/PhoneNumberUtils.cpp b/android/PhoneNumberUtils.cpp
index d469ed7..68c0fb6 100644
--- a/android/PhoneNumberUtils.cpp
+++ b/android/PhoneNumberUtils.cpp
@@ -138,7 +138,8 @@
* digit to compare two phone numbers.
*/
static int tryGetCountryCallingCode(const char *str, size_t len,
- const char **new_ptr, size_t *new_len)
+ const char **new_ptr, size_t *new_len,
+ bool accept_thailand_case)
{
// Rough regexp:
// ^[^0-9*#+]*((\+|0(0|11)\d\d?|166) [^0-9*#+] $
@@ -155,8 +156,13 @@
case 0:
if (ch == '+') state = 1;
else if (ch == '0') state = 2;
- else if (ch == '1') state = 8;
- else if (isDialable(ch)) return -1;
+ else if (ch == '1') {
+ if (accept_thailand_case) {
+ state = 8;
+ } else {
+ return -1;
+ }
+ } else if (isDialable(ch)) return -1;
break;
case 2:
@@ -272,8 +278,11 @@
* In the future, we should handle trunk prefix more correctly, but as of now,
* we just ignore it...
*/
-bool phone_number_compare(const char* a, const char* b)
+static bool phone_number_compare_inter(const char* const org_a, const char* const org_b,
+ bool accept_thailand_case)
{
+ const char* a = org_a;
+ const char* b = org_b;
size_t len_a = 0;
size_t len_b = 0;
if (a == NULL) {
@@ -292,9 +301,12 @@
size_t tmp_len_a = len_a;
size_t tmp_len_b = len_b;
- int ccc_a = tryGetCountryCallingCode(a, len_a, &tmp_a, &tmp_len_a);
- int ccc_b = tryGetCountryCallingCode(b, len_b, &tmp_b, &tmp_len_b);
+ int ccc_a = tryGetCountryCallingCode(a, len_a, &tmp_a, &tmp_len_a, accept_thailand_case);
+ int ccc_b = tryGetCountryCallingCode(b, len_b, &tmp_b, &tmp_len_b, accept_thailand_case);
+ bool both_have_ccc = false;
bool ok_to_ignore_prefix = true;
+ bool trunk_prefix_is_omitted_a = false;
+ bool trunk_prefix_is_omitted_b = false;
if (ccc_a >= 0 && ccc_b >= 0) {
if (ccc_a != ccc_b) {
// Different Country Calling Code. Must be different phone number.
@@ -303,6 +315,7 @@
// When both have ccc, do not ignore trunk prefix. Without this,
// "+81123123" becomes same as "+810123123" (+81 == Japan)
ok_to_ignore_prefix = false;
+ both_have_ccc = true;
} else if (ccc_a < 0 && ccc_b < 0) {
// When both do not have ccc, do not ignore trunk prefix. Without this,
// "123123" becomes same as "0123123"
@@ -310,9 +323,11 @@
} else {
if (ccc_a < 0) {
tryGetTrunkPrefixOmittedStr(a, len_a, &tmp_a, &tmp_len_a);
+ trunk_prefix_is_omitted_a = true;
}
if (ccc_b < 0) {
tryGetTrunkPrefixOmittedStr(b, len_b, &tmp_b, &tmp_len_b);
+ trunk_prefix_is_omitted_b = true;
}
}
@@ -350,21 +365,42 @@
}
if (ok_to_ignore_prefix) {
- if (!checkPrefixIsIgnorable(a, i_a)) {
- return false;
+ if ((trunk_prefix_is_omitted_a && i_a >= 0) ||
+ !checkPrefixIsIgnorable(a, i_a)) {
+ if (accept_thailand_case) {
+ // Maybe the code handling the special case for Thailand makes the
+ // result garbled, so disable the code and try again.
+ // e.g. "16610001234" must equal to "6610001234", but with
+ // Thailand-case handling code, they become equal to each other.
+ //
+ // Note: we select simplicity rather than adding some complicated
+ // logic here for performance(like "checking whether remaining
+ // numbers are just 66 or not"), assuming inputs are small
+ // enough.
+ return phone_number_compare_inter(org_a, org_b, false);
+ } else {
+ return false;
+ }
}
- if (!checkPrefixIsIgnorable(b, i_b)) {
- return false;
+ if ((trunk_prefix_is_omitted_b && i_b >= 0) ||
+ !checkPrefixIsIgnorable(b, i_b)) {
+ if (accept_thailand_case) {
+ return phone_number_compare_inter(org_a, org_b, false);
+ } else {
+ return false;
+ }
}
} else {
// In the US, 1-650-555-1234 must be equal to 650-555-1234,
// while 090-1234-1234 must not be equalt to 90-1234-1234 in Japan.
// This request exists just in US (with 1 trunk (NDD) prefix).
+ // In addition, "011 11 7005554141" must not equal to "+17005554141",
+ // while "011 1 7005554141" must equal to "+17005554141"
//
- // At least, in this "rough" comparison, we should ignore the prefix
- // '1', so if the remaining non-separator number is 0, we ignore it
- // just once.
- bool may_be_namp = true;
+ // In this comparison, we ignore the prefix '1' just once, when
+ // - at least either does not have CCC, or
+ // - the remaining non-separator number is 1
+ bool may_be_namp = !both_have_ccc;
while (i_a >= 0) {
const char ch_a = a[i_a];
if (isDialable(ch_a)) {
@@ -392,4 +428,9 @@
return true;
}
+bool phone_number_compare(const char* a, const char* b)
+{
+ return phone_number_compare_inter(a, b, true);
+}
+
} // namespace android
diff --git a/android/PhoneNumberUtilsTest.cpp b/android/PhoneNumberUtilsTest.cpp
index a33f10a..4d9809d 100644
--- a/android/PhoneNumberUtilsTest.cpp
+++ b/android/PhoneNumberUtilsTest.cpp
@@ -82,6 +82,7 @@
EXPECT_EQ("650-253-0000", " 1-650-253-0000");
EXPECT_NE("650-253-0000", "11-650-253-0000");
EXPECT_NE("650-253-0000", "0-650-253-0000");
+ EXPECT_EQ("555-4141", "+1-700-555-4141");
EXPECT_EQ("+1 650-253-0000", "6502530000");
EXPECT_EQ("001 650-253-0000", "6502530000");
@@ -124,6 +125,10 @@
// Confirm that the bug found before does not re-appear.
EXPECT_NE("080-1234-5678", "+819012345678");
EXPECT_EQ("650-000-3456", "16500003456");
+ EXPECT_EQ("011 1 7005554141", "+17005554141");
+ EXPECT_NE("011 11 7005554141", "+17005554141");
+ EXPECT_NE("+44 207 792 3490", "00 207 792 3490");
+ // This is not related to Thailand case. NAMP "1" + region code "661".
EXPECT_EQ("16610001234", "6610001234");
// We also need to compare two alpha addresses to make sure two different strings