ex-phone-number / ex_phone_number

Elixir port of libphonenumber
MIT License
251 stars 61 forks source link

Document parsing E164 formatted numbers #35

Closed sushant12 closed 4 years ago

sushant12 commented 4 years ago

It would be nice if this lib could parse ExPhoneNumber.parse("+977123456789") without passing the country.

sushant12 commented 4 years ago

it turns out if I pass E164 formatted number and an arbitrary string then it can extract its country code

szymon-jez commented 4 years ago

@sushant12 Thanks for making the effort to check this and sharing here. I see that this is not documented, the API does also not make this clear. I assume that you have found out about this by experimentation and/or reading the code. I think that documenting this (e.g. providing an example usage) would be beneficial. A PR with such is welcome.

sushant12 commented 4 years ago

Hi @szymon-jez

Instead of passing an arbitrary string as the second parameter in parse/2 , should not it accept single parameter for e164 formatter number? eg: ExPhoneNumber.parse("+977123456789")

szymon-jez commented 4 years ago

Yes, calling the function with "" or nil as the second argument can feel awkward. Therefore adding ExPhoneNumber.parse_e164("+977123456789") could be an explicit way to deal with this.

An alternative – to not widen the API – would be to document how the parse function deals with e164 numbers. Here are a few examples how it behaves:

iex(46)> {:ok, phone_number} = ExPhoneNumber.parse("+48 123456789", "PL")
{:ok,
 %ExPhoneNumber.Model.PhoneNumber{
   country_code: 48,
   country_code_source: nil,
   extension: nil,
   italian_leading_zero: nil,
   national_number: 123456789,
   number_of_leading_zeros: nil,
   preferred_domestic_carrier_code: nil,
   raw_input: nil
 }}
iex(47)> {:ok, phone_number} = ExPhoneNumber.parse("+48 123456789", "")  
{:ok,
 %ExPhoneNumber.Model.PhoneNumber{
   country_code: 48,
   country_code_source: nil,
   extension: nil,
   italian_leading_zero: nil,
   national_number: 123456789,
   number_of_leading_zeros: nil,
   preferred_domestic_carrier_code: nil,
   raw_input: nil
 }}
iex(48)> {:ok, phone_number} = ExPhoneNumber.parse("+48 123456789", nil) 
{:ok,
 %ExPhoneNumber.Model.PhoneNumber{
   country_code: 48,
   country_code_source: nil,
   extension: nil,
   italian_leading_zero: nil,
   national_number: 123456789,
   number_of_leading_zeros: nil,
   preferred_domestic_carrier_code: nil,
   raw_input: nil
 }}
iex(49)> {:ok, phone_number} = ExPhoneNumber.parse("+48 123456789", "GB") 
{:ok,
 %ExPhoneNumber.Model.PhoneNumber{
   country_code: 48,
   country_code_source: nil,
   extension: nil,
   italian_leading_zero: nil,
   national_number: 123456789,
   number_of_leading_zeros: nil,
   preferred_domestic_carrier_code: nil,
   raw_input: nil
 }}
iex(50)> {:ok, phone_number} = ExPhoneNumber.parse("+48 123", "PL") # invalid     
{:ok,
 %ExPhoneNumber.Model.PhoneNumber{
   country_code: 48,
   country_code_source: nil,
   extension: nil,
   italian_leading_zero: nil,
   national_number: 123,
   number_of_leading_zeros: nil,
   preferred_domestic_carrier_code: nil,
   raw_input: nil
 }}

So parsing an e164 number does not even look at the provided country code.