From adf9548a494486369f1fa7bb0ff37b52cf72e7f3 Mon Sep 17 00:00:00 2001 From: Andrey Koleshko Date: Fri, 29 Jan 2016 15:10:05 +0300 Subject: [PATCH 1/6] Hide parse_international_string --- lib/global_phone/parsing.rb | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/lib/global_phone/parsing.rb b/lib/global_phone/parsing.rb index 1aa88b0..d9fb7f4 100644 --- a/lib/global_phone/parsing.rb +++ b/lib/global_phone/parsing.rb @@ -18,16 +18,17 @@ def parse(string, territory_name) end end - def parse_international_string(string) - string = Number.normalize(string) - string = strip_leading_plus(string) if starts_with_plus?(string) + private + + def parse_international_string(string) + string = Number.normalize(string) + string = strip_leading_plus(string) if starts_with_plus?(string) - if region = region_for_string(string) - region.parse_national_string(string) + if region = region_for_string(string) + region.parse_national_string(string) + end end - end - protected def starts_with_plus?(string) string[0, 1] == "+" end From e1ba2501d3edf6e192bd2cc9bea71999a436e24c Mon Sep 17 00:00:00 2001 From: Andrey Koleshko Date: Fri, 29 Jan 2016 15:15:38 +0300 Subject: [PATCH 2/6] Do not spread interface for one class to modules --- lib/global_phone.rb | 1 + lib/global_phone/database.rb | 63 ++++++++++++++++++++++++++++++------ lib/global_phone/parsing.rb | 53 ------------------------------ lib/global_phone/region.rb | 1 - 4 files changed, 54 insertions(+), 64 deletions(-) delete mode 100644 lib/global_phone/parsing.rb diff --git a/lib/global_phone.rb b/lib/global_phone.rb index 49339e8..1949bbf 100644 --- a/lib/global_phone.rb +++ b/lib/global_phone.rb @@ -1,3 +1,4 @@ +require 'global_phone/utils' require 'global_phone/context' module GlobalPhone diff --git a/lib/global_phone/database.rb b/lib/global_phone/database.rb index a1f27a3..fbdf342 100644 --- a/lib/global_phone/database.rb +++ b/lib/global_phone/database.rb @@ -1,9 +1,8 @@ -require 'global_phone/parsing' require 'global_phone/region' module GlobalPhone class Database - include Parsing + attr_reader :regions def self.load_file(filename) load(File.read(filename)) @@ -14,13 +13,26 @@ def self.load(json) new(JSON.parse(json)) end - attr_reader :regions - def initialize(record_data) @regions = record_data.map { |data| Region.new(data) } @territories_by_name = {} end + def parse(string, territory_name) + string = Number.normalize(string) + territory = self.territory(territory_name) + raise ArgumentError, "unknown territory `#{territory_name}'" unless territory + + if starts_with_plus?(string) + parse_international_string(string) + elsif string =~ territory.international_prefix + string = strip_international_prefix(territory, string) + parse_international_string(string) + else + territory.parse_national_string(string) + end + end + def region(country_code) regions_by_country_code[country_code.to_s] end @@ -36,13 +48,44 @@ def inspect "#<#{self.class.name}>" end - protected - def regions_by_country_code - @regions_by_country_code ||= Hash[*regions.map { |r| [r.country_code, r] }.flatten] - end + private + + def parse_international_string(string) + string = Number.normalize(string) + string = strip_leading_plus(string) if starts_with_plus?(string) - def region_for_territory(name) - regions.find { |r| r.has_territory?(name) } + if region = region_for_string(string) + region.parse_national_string(string) end + end + + def starts_with_plus?(string) + string[0, 1] == "+" + end + + def strip_leading_plus(string) + string[1..-1] + end + + def strip_international_prefix(territory, string) + string.sub(territory.international_prefix, "") + end + + def region_for_string(string) + candidates = country_code_candidates_for(string) + Utils.map_detect(candidates) { |country_code| region(country_code) } + end + + def country_code_candidates_for(string) + (1..3).map { |length| string[0, length] } + end + + def regions_by_country_code + @regions_by_country_code ||= Hash[*regions.map { |r| [r.country_code, r] }.flatten] + end + + def region_for_territory(name) + regions.find { |r| r.has_territory?(name) } + end end end diff --git a/lib/global_phone/parsing.rb b/lib/global_phone/parsing.rb deleted file mode 100644 index d9fb7f4..0000000 --- a/lib/global_phone/parsing.rb +++ /dev/null @@ -1,53 +0,0 @@ -require 'global_phone/number' -require 'global_phone/utils' - -module GlobalPhone - module Parsing - def parse(string, territory_name) - string = Number.normalize(string) - territory = self.territory(territory_name) - raise ArgumentError, "unknown territory `#{territory_name}'" unless territory - - if starts_with_plus?(string) - parse_international_string(string) - elsif string =~ territory.international_prefix - string = strip_international_prefix(territory, string) - parse_international_string(string) - else - territory.parse_national_string(string) - end - end - - private - - def parse_international_string(string) - string = Number.normalize(string) - string = strip_leading_plus(string) if starts_with_plus?(string) - - if region = region_for_string(string) - region.parse_national_string(string) - end - end - - def starts_with_plus?(string) - string[0, 1] == "+" - end - - def strip_leading_plus(string) - string[1..-1] - end - - def strip_international_prefix(territory, string) - string.sub(territory.international_prefix, "") - end - - def region_for_string(string) - candidates = country_code_candidates_for(string) - Utils.map_detect(candidates) { |country_code| region(country_code) } - end - - def country_code_candidates_for(string) - (1..3).map { |length| string[0, length] } - end - end -end diff --git a/lib/global_phone/region.rb b/lib/global_phone/region.rb index 813ec37..c02c3a5 100644 --- a/lib/global_phone/region.rb +++ b/lib/global_phone/region.rb @@ -1,7 +1,6 @@ require 'global_phone/format' require 'global_phone/record' require 'global_phone/territory' -require 'global_phone/utils' module GlobalPhone class Region < Record From 853691b79555839c468c4257c487c83e3ef4bec4 Mon Sep 17 00:00:00 2001 From: Andrey Koleshko Date: Fri, 29 Jan 2016 15:20:19 +0300 Subject: [PATCH 3/6] Use private instead of protected --- lib/global_phone/database_generator.rb | 2 +- lib/global_phone/number.rb | 2 +- lib/global_phone/region.rb | 2 +- lib/global_phone/territory.rb | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/global_phone/database_generator.rb b/lib/global_phone/database_generator.rb index 959edad..de49393 100644 --- a/lib/global_phone/database_generator.rb +++ b/lib/global_phone/database_generator.rb @@ -35,7 +35,7 @@ def inspect "#<#{self.class.name} (#{doc.search("*").size} elements)>" end - protected + private def territory_nodes doc.search("territory") end diff --git a/lib/global_phone/number.rb b/lib/global_phone/number.rb index 7f209a5..71f78e6 100644 --- a/lib/global_phone/number.rb +++ b/lib/global_phone/number.rb @@ -65,7 +65,7 @@ def to_s international_string end - protected + private def format @format ||= find_format_for(national_string) end diff --git a/lib/global_phone/region.rb b/lib/global_phone/region.rb index c02c3a5..86c8193 100644 --- a/lib/global_phone/region.rb +++ b/lib/global_phone/region.rb @@ -41,7 +41,7 @@ def inspect "#<#{self.class.name} country_code=#{country_code} territories=[#{territory_names.join(",")}]>" end - protected + private def territory_names territory_record_data.map(&:first) end diff --git a/lib/global_phone/territory.rb b/lib/global_phone/territory.rb index 3dd95b0..514e051 100644 --- a/lib/global_phone/territory.rb +++ b/lib/global_phone/territory.rb @@ -30,7 +30,7 @@ def inspect "#<#{self.class.name} country_code=#{country_code} name=#{name}>" end - protected + private def strip_prefixes(string) if national_prefix_for_parsing transform_rule = national_prefix_transform_rule || "" From f5c0758729916e2bb69b224c5b98728f5c5a2354 Mon Sep 17 00:00:00 2001 From: Andrey Koleshko Date: Fri, 29 Jan 2016 15:51:22 +0300 Subject: [PATCH 4/6] Move require to the file context --- lib/global_phone/database.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/global_phone/database.rb b/lib/global_phone/database.rb index fbdf342..8623cf9 100644 --- a/lib/global_phone/database.rb +++ b/lib/global_phone/database.rb @@ -1,3 +1,4 @@ +require 'json' require 'global_phone/region' module GlobalPhone @@ -9,7 +10,6 @@ def self.load_file(filename) end def self.load(json) - require 'json' new(JSON.parse(json)) end From 893d3668836d97d1612f61c58877382510217f05 Mon Sep 17 00:00:00 2001 From: Andrey Koleshko Date: Fri, 29 Jan 2016 16:42:53 +0300 Subject: [PATCH 5/6] Allow to find a territory by country code --- lib/global_phone.rb | 1 - lib/global_phone/context.rb | 6 ++++++ lib/global_phone/database.rb | 23 +++++++++++++++-------- lib/global_phone/region.rb | 6 +++++- lib/global_phone/utils.rb | 14 -------------- test/context_test.rb | 24 +++++++++++++++++++++++- 6 files changed, 49 insertions(+), 25 deletions(-) delete mode 100644 lib/global_phone/utils.rb diff --git a/lib/global_phone.rb b/lib/global_phone.rb index 1949bbf..49339e8 100644 --- a/lib/global_phone.rb +++ b/lib/global_phone.rb @@ -1,4 +1,3 @@ -require 'global_phone/utils' require 'global_phone/context' module GlobalPhone diff --git a/lib/global_phone/context.rb b/lib/global_phone/context.rb index 4524524..561fe9a 100644 --- a/lib/global_phone/context.rb +++ b/lib/global_phone/context.rb @@ -23,6 +23,12 @@ def parse(string, territory_name = default_territory_name) db.parse(string, territory_name) end + def territory_for_string(string) + region = db.region_for_string(string) + return unless region + region.territory_for_contry_code + end + def normalize(string, territory_name = default_territory_name) number = parse(string, territory_name) number.international_string if number diff --git a/lib/global_phone/database.rb b/lib/global_phone/database.rb index 8623cf9..1172535 100644 --- a/lib/global_phone/database.rb +++ b/lib/global_phone/database.rb @@ -48,11 +48,19 @@ def inspect "#<#{self.class.name}>" end + def region_for_string(string) + country_code_candidates_for(strip_leading_plus(string)).each do |country_code| + if found_region = region(country_code) + return found_region + end + end + nil + end + private def parse_international_string(string) - string = Number.normalize(string) - string = strip_leading_plus(string) if starts_with_plus?(string) + string = strip_leading_plus(Number.normalize(string)) if region = region_for_string(string) region.parse_national_string(string) @@ -64,18 +72,17 @@ def starts_with_plus?(string) end def strip_leading_plus(string) - string[1..-1] + if starts_with_plus?(string) + string[1..-1] + else + string + end end def strip_international_prefix(territory, string) string.sub(territory.international_prefix, "") end - def region_for_string(string) - candidates = country_code_candidates_for(string) - Utils.map_detect(candidates) { |country_code| region(country_code) } - end - def country_code_candidates_for(string) (1..3).map { |length| string[0, length] } end diff --git a/lib/global_phone/region.rb b/lib/global_phone/region.rb index 86c8193..83f9772 100644 --- a/lib/global_phone/region.rb +++ b/lib/global_phone/region.rb @@ -22,7 +22,7 @@ def territories def territory(name) name = name.to_s.upcase - territories.detect { |region| region.name == name } + territories.detect { |territory| territory.name == name } end def has_territory?(name) @@ -37,6 +37,10 @@ def parse_national_string(string) end end + def territory_for_contry_code + territories.first + end + def inspect "#<#{self.class.name} country_code=#{country_code} territories=[#{territory_names.join(",")}]>" end diff --git a/lib/global_phone/utils.rb b/lib/global_phone/utils.rb deleted file mode 100644 index 778203e..0000000 --- a/lib/global_phone/utils.rb +++ /dev/null @@ -1,14 +0,0 @@ -module GlobalPhone - module Utils - extend self - - def map_detect(collection) - collection.each do |value| - if result = yield(value) - return result - end - end - nil - end - end -end diff --git a/test/context_test.rb b/test/context_test.rb index 637b62d..d33d216 100644 --- a/test/context_test.rb +++ b/test/context_test.rb @@ -71,11 +71,33 @@ class ContextTest < TestCase assert_nil context.validate("+0651816068") end + test 'finding region by string' do + assert_found_territory_for_string('1', {country_code: '1', country_name: 'US'}) + assert_found_territory_for_string('+1', {country_code: '1', country_name: 'US'}) + assert_found_territory_for_string('131', {country_code: '1', country_name: 'US'}) + assert_found_territory_for_string('+375', {country_code: '375', country_name: 'BY'}) + + assert_not_found_territory_for_string('+9') + end + def assert_parses(string, assertions) territory_name = assertions.delete(:with_territory) || context.default_territory_name number = context.parse(string, territory_name) assert_kind_of Number, number - assert_equal({ :country_code => number.country_code, :national_string => number.national_string }, assertions) + assert_equal(assertions, + { :country_code => number.country_code, :national_string => number.national_string }) + end + + def assert_found_territory_for_string(string, assertions) + territory = context.territory_for_string(string) + + assert_equal(assertions, {country_code: territory.country_code, country_name: territory.name}) + end + + def assert_not_found_territory_for_string(string) + territory = context.territory_for_string(string) + + assert_nil territory end def assert_does_not_parse(string, options = {}) From e75319ed5b5a1430e034ec9309d94a948f282a7e Mon Sep 17 00:00:00 2001 From: Andrey Koleshko Date: Fri, 29 Jan 2016 16:55:31 +0300 Subject: [PATCH 6/6] Rename region_for_string to region_for_country_code --- lib/global_phone/context.rb | 4 ++-- lib/global_phone/database.rb | 4 ++-- test/context_test.rb | 18 +++++++++--------- 3 files changed, 13 insertions(+), 13 deletions(-) diff --git a/lib/global_phone/context.rb b/lib/global_phone/context.rb index 561fe9a..dc0d066 100644 --- a/lib/global_phone/context.rb +++ b/lib/global_phone/context.rb @@ -23,8 +23,8 @@ def parse(string, territory_name = default_territory_name) db.parse(string, territory_name) end - def territory_for_string(string) - region = db.region_for_string(string) + def territory_for_country_code(string) + region = db.region_for_country_code(string) return unless region region.territory_for_contry_code end diff --git a/lib/global_phone/database.rb b/lib/global_phone/database.rb index 1172535..a4fd197 100644 --- a/lib/global_phone/database.rb +++ b/lib/global_phone/database.rb @@ -48,7 +48,7 @@ def inspect "#<#{self.class.name}>" end - def region_for_string(string) + def region_for_country_code(string) country_code_candidates_for(strip_leading_plus(string)).each do |country_code| if found_region = region(country_code) return found_region @@ -62,7 +62,7 @@ def region_for_string(string) def parse_international_string(string) string = strip_leading_plus(Number.normalize(string)) - if region = region_for_string(string) + if region = region_for_country_code(string) region.parse_national_string(string) end end diff --git a/test/context_test.rb b/test/context_test.rb index d33d216..b84b58c 100644 --- a/test/context_test.rb +++ b/test/context_test.rb @@ -72,12 +72,12 @@ class ContextTest < TestCase end test 'finding region by string' do - assert_found_territory_for_string('1', {country_code: '1', country_name: 'US'}) - assert_found_territory_for_string('+1', {country_code: '1', country_name: 'US'}) - assert_found_territory_for_string('131', {country_code: '1', country_name: 'US'}) - assert_found_territory_for_string('+375', {country_code: '375', country_name: 'BY'}) + assert_found_territory_for_country_code('1', {country_code: '1', country_name: 'US'}) + assert_found_territory_for_country_code('+1', {country_code: '1', country_name: 'US'}) + assert_found_territory_for_country_code('131', {country_code: '1', country_name: 'US'}) + assert_found_territory_for_country_code('+375', {country_code: '375', country_name: 'BY'}) - assert_not_found_territory_for_string('+9') + assert_not_found_territory_for_country_code('+9') end def assert_parses(string, assertions) @@ -88,14 +88,14 @@ def assert_parses(string, assertions) { :country_code => number.country_code, :national_string => number.national_string }) end - def assert_found_territory_for_string(string, assertions) - territory = context.territory_for_string(string) + def assert_found_territory_for_country_code(string, assertions) + territory = context.territory_for_country_code(string) assert_equal(assertions, {country_code: territory.country_code, country_name: territory.name}) end - def assert_not_found_territory_for_string(string) - territory = context.territory_for_string(string) + def assert_not_found_territory_for_country_code(string) + territory = context.territory_for_country_code(string) assert_nil territory end