search

DrHyde / perl-modules-Number-Phone

Number::Phone and friends
24 stars 32 forks source link

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

Open mikeraynham opened 6 years ago

mikeraynham commented 6 years ago

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.

coveralls commented 6 years ago

Coverage Status

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

DrHyde commented 6 years ago

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 commented 6 years ago

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.