Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make handling of UK country code consistent between Number::Phone::UK and Number::Phone::Lib #88

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

mikeraynham
Copy link

When given both an alpha-2 country code, and a number with an international country calling code prefix, the behaviour of Number::Phone::UK and Number::Phone::Lib is inconsistent.

Number::Phone::Lib strips the "+" from the number, causing the alpha-2 country code to take priority. For example, "+238 989 12 34" is a valid Cape Verde number. Removing the "+" prefix gives a valid UK number (albeit without the leading zero). Given "GB" and "+238 989 12 34", Number::Phone::Lib will return a UK object for "+44 23 8989 1234", whereas Number::Phone will return undef, because the number fails the basic UK number length check.

Given "+238 989 12 3", which is an invalid Cape Verde number, and an invalid UK number, Number::Phone::Lib will return undef. Number::Phone, however, returns a Number::Phone::UK object, which when formatted returns "+44+238989123".

This PR causes Number::Phone::UK to behave in the same way as Number::Phone::Lib.

When given both an alpha-2 country code, and a number with an
international country calling code prefix, the behaviour of
Number::Phone::UK and Number::Phone::Lib is inconsistent.
When given both an alpha-2 country code, and a number with an
international country calling code prefix, the behaviour of
Number::Phone::UK and Number::Phone::Lib is inconsistent.

Number::Phone::Lib strips the "+" from the number, causing the alpha-2
country code to take priority.  For example, "+238 989 12 34" is a valid
Cape Verde number.  Removing the "+" prefix gives a valid UK number
(albeit without the leading zero).  Given "GB" and "+238 989 12 34",
Number::Phone::Lib will return a UK object for "+44 23 8989 1234",
whereas Number::Phone will return undef, because the number fails the
basic UK number length check.

Given "+238 989 12 3", which is an invalid Cape Verde number, and an
invalid UK number, Number::Phone::Lib will return undef.  Number::Phone,
however, returns a Number::Phone::UK object, which when formatted,
returns "+44+238989123".

This commit makes Number::Phone::UK behaviour consistent with that of
Number::Phone::Lib.
@coveralls
Copy link

Coverage Status

Coverage increased (+2.0e-05%) to 99.938% when pulling d6cf761 on mikeraynham:uk_country_code into 59b1fe1 on DrHyde:master.

@DrHyde
Copy link
Owner

DrHyde commented Jul 10, 2018

Both Number::Phone and Number::Phone::Lib are wrong then. Thanks for drawing this to my attention! However, the correct fix is not to just make one of them behave like the other, but to make them both behave correctly. The correct behaviour would be for incompatible country codes to just throw an error and not try to be clever. Trouble is, this is an incompatible change that will break code out there and I value stability in this code very highly so it needs to just emit warnings for a while before being removed. I've never documented how long a deprecation cycle is, but in the past it's been at least a year.

@mikeraynham
Copy link
Author

Hi,

Thanks for looking into this, and sorry for the late response. I agree that removing the magic, and throwing an error where incompatible values are supplied, is indeed a better fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants