From 9c5448dcbd1b17d411998c8effe7974ab5bbd0aa Mon Sep 17 00:00:00 2001 From: Keghani Kouzoujian Date: Fri, 30 Jun 2017 18:41:30 +0200 Subject: [PATCH] Update javadoc on isNumberGeographical (#1823) --- cpp/src/phonenumbers/phonenumberutil.cc | 8 ++++---- cpp/src/phonenumbers/phonenumberutil.h | 9 +++------ cpp/src/phonenumbers/regex_based_matcher.h | 4 ++-- .../google/i18n/phonenumbers/PhoneNumberUtil.java | 15 ++++++--------- javascript/i18n/phonenumbers/phonenumberutil.js | 6 +++--- pending_code_changes.txt | 1 + 6 files changed, 19 insertions(+), 24 deletions(-) diff --git a/cpp/src/phonenumbers/phonenumberutil.cc b/cpp/src/phonenumbers/phonenumberutil.cc index 9eba5aa19..18f20afa1 100644 --- a/cpp/src/phonenumbers/phonenumberutil.cc +++ b/cpp/src/phonenumbers/phonenumberutil.cc @@ -2364,12 +2364,12 @@ bool PhoneNumberUtil::IsNumberGeographical( } bool PhoneNumberUtil::IsNumberGeographical( - PhoneNumberType number_type, int country_calling_code) const { - return number_type == PhoneNumberUtil::FIXED_LINE || - number_type == PhoneNumberUtil::FIXED_LINE_OR_MOBILE || + PhoneNumberType phone_number_type, int country_calling_code) const { + return phone_number_type == PhoneNumberUtil::FIXED_LINE || + phone_number_type == PhoneNumberUtil::FIXED_LINE_OR_MOBILE || (reg_exps_->geo_mobile_countries_.find(country_calling_code) != reg_exps_->geo_mobile_countries_.end() && - number_type == PhoneNumberUtil::MOBILE); + phone_number_type == PhoneNumberUtil::MOBILE); } // A helper function to set the values related to leading zeros in a diff --git a/cpp/src/phonenumbers/phonenumberutil.h b/cpp/src/phonenumbers/phonenumberutil.h index b0cf5cc8e..870e6f05b 100644 --- a/cpp/src/phonenumbers/phonenumberutil.h +++ b/cpp/src/phonenumbers/phonenumberutil.h @@ -591,14 +591,11 @@ class PhoneNumberUtil : public Singleton { bool CanBeInternationallyDialled(const PhoneNumber& number) const; // Tests whether a phone number has a geographical association. It checks if - // the number is associated to a certain region in the country where it - // belongs to. Note that this doesn't verify if the number is actually in use. + // the number is associated with a certain region in the country to which it + // belongs. Note that this doesn't verify if the number is actually in use. bool IsNumberGeographical(const PhoneNumber& phone_number) const; - // Tests whether a phone number has a geographical association, as represented - // by its type and the country it belongs to. - // - // This version of IsNumberGeographical exists since calculating the phone + // Overload of IsNumberGeographical(PhoneNumber), since calculating the phone // number type is expensive; if we have already done this, we don't want to do // it again. bool IsNumberGeographical(PhoneNumberType phone_number_type, diff --git a/cpp/src/phonenumbers/regex_based_matcher.h b/cpp/src/phonenumbers/regex_based_matcher.h index 81b49efca..a375cb6e4 100644 --- a/cpp/src/phonenumbers/regex_based_matcher.h +++ b/cpp/src/phonenumbers/regex_based_matcher.h @@ -37,12 +37,12 @@ class RegexBasedMatcher : public MatcherApi { RegexBasedMatcher(); ~RegexBasedMatcher(); - bool MatchNationalNumber(const string& national_number, + bool MatchNationalNumber(const string& number, const PhoneNumberDesc& number_desc, bool allow_prefix_match) const; private: - bool Match(const string& national_number, const string& number_pattern, + bool Match(const string& number, const string& number_pattern, bool allow_prefix_match) const; const scoped_ptr regexp_factory_; diff --git a/java/libphonenumber/src/com/google/i18n/phonenumbers/PhoneNumberUtil.java b/java/libphonenumber/src/com/google/i18n/phonenumbers/PhoneNumberUtil.java index bcf511b85..239138c56 100644 --- a/java/libphonenumber/src/com/google/i18n/phonenumbers/PhoneNumberUtil.java +++ b/java/libphonenumber/src/com/google/i18n/phonenumbers/PhoneNumberUtil.java @@ -1124,7 +1124,7 @@ public class PhoneNumberUtil { /** * Tests whether a phone number has a geographical association. It checks if the number is - * associated to a certain region in the country where it belongs to. Note that this doesn't + * associated with a certain region in the country to which it belongs. Note that this doesn't * verify if the number is actually in use. */ public boolean isNumberGeographical(PhoneNumber phoneNumber) { @@ -1132,17 +1132,14 @@ public class PhoneNumberUtil { } /** - * Tests whether a phone number has a geographical association, as represented by its type and the - * country it belongs to. - * - * This version of isNumberGeographical exists since calculating the phone number type is + * Overload of isNumberGeographical(PhoneNumber), since calculating the phone number type is * expensive; if we have already done this, we don't want to do it again. */ - public boolean isNumberGeographical(PhoneNumberType numberType, int countryCallingCode) { - return numberType == PhoneNumberType.FIXED_LINE - || numberType == PhoneNumberType.FIXED_LINE_OR_MOBILE + public boolean isNumberGeographical(PhoneNumberType phoneNumberType, int countryCallingCode) { + return phoneNumberType == PhoneNumberType.FIXED_LINE + || phoneNumberType == PhoneNumberType.FIXED_LINE_OR_MOBILE || (GEO_MOBILE_COUNTRIES.contains(countryCallingCode) - && numberType == PhoneNumberType.MOBILE); + && phoneNumberType == PhoneNumberType.MOBILE); } /** diff --git a/javascript/i18n/phonenumbers/phonenumberutil.js b/javascript/i18n/phonenumbers/phonenumberutil.js index 702341f15..6fe0a75d2 100644 --- a/javascript/i18n/phonenumbers/phonenumberutil.js +++ b/javascript/i18n/phonenumbers/phonenumberutil.js @@ -1531,9 +1531,9 @@ i18n.phonenumbers.PhoneNumberUtil.prototype.formattingRuleHasFirstGroupOnly = /** - * Tests whether a phone number has a geographical association. It checks if - * the number is associated to a certain region in the country where it belongs - * to. Note that this doesn't verify if the number is actually in use. + * Tests whether a phone number has a geographical association. It checks if the + * number is associated with a certain region in the country to which it + * belongs. Note that this doesn't verify if the number is actually in use. * * @param {i18n.phonenumbers.PhoneNumber} phoneNumber The phone number to test. * @return {boolean} true if the phone number has a geographical association. diff --git a/pending_code_changes.txt b/pending_code_changes.txt index 08e1a6572..3db26f902 100644 --- a/pending_code_changes.txt +++ b/pending_code_changes.txt @@ -9,3 +9,4 @@ Code changes: starting with 0s may be noticed in some countries. - Bug fix for Javascript: getNationalSignificantNumber used to print the string "null" for empty phone number objects, now it returns an empty string. + - Updated the documentation for the isNumberGeographical API.