Commit b3b8a9da authored by Daisuke Miyakawa's avatar Daisuke Miyakawa
Browse files

Fix the implementation of phone_number_compare in accordance with the tests in...

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...
parent db6e3c88
......@@ -40,9 +40,13 @@ include $(BUILD_EXECUTABLE)
# 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
......
......@@ -138,7 +138,8 @@ static bool tryGetTrunkPrefixOmittedStr(const char *str, size_t len,
* 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 @@ static int tryGetCountryCallingCode(const char *str, size_t len,
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 @@ static bool checkPrefixIsIgnorable(const char* ch, int i) {
* 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 @@ bool phone_number_compare(const char* a, const char* b)
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 @@ bool phone_number_compare(const char* a, const char* b)
// 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 @@ bool phone_number_compare(const char* a, const char* b)
} 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 @@ bool phone_number_compare(const char* a, const char* b)
}
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 @@ bool phone_number_compare(const char* a, const char* b)
return true;
}
bool phone_number_compare(const char* a, const char* b)
{
return phone_number_compare_inter(a, b, true);
}
} // namespace android
......@@ -82,6 +82,7 @@ int main() {
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 @@ int main() {
// 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
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment