DrHyde / perl-modules-Number-Phone

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

Add national formatting of numbers. #83

Closed dracos closed 6 years ago

dracos commented 6 years ago

This adds a new National formatter that outputs the number in national format, using stored nationalPrefixFormattingRule to work out what to display.

It also adds a format_for_country function that will then return national format if in same country, international otherwise.

As well as standard usage such as being able to show UK numbers as 020...., one important example is Argentina, where the international number +54 9 11 1234-5678 is the national number 011 15-1234-5678 (9 removed, 15 is added between area code and local code).

(This builds upon #80, with one commit on top of that PR.) Fixes #79.

coveralls commented 6 years ago

Coverage Status

Coverage increased (+0.0002%) to 99.967% when pulling b0dbf84518b786faf202264c570eec747468c3cc on dracos:79-national-formatting into 850ad3185171a00c5962b73dfc169ce08f1cab9a on DrHyde:master.

dracos commented 6 years ago

~~I've just had a thought - you might well want to provide a country code/+number to the national function, or a different function (similar to how it's provided to new()), and then it would return the national number if it was for that country, or the international number otherwise. Does that sound good? Update: Have changed how this is done, using an argument format, to allow this.~~

coveralls commented 6 years ago

Coverage Status

Coverage increased (+0.0002%) to 99.967% when pulling ed2909edcbb6382510f32c93f5d5e07b2a3e67bc on dracos:79-national-formatting into 850ad3185171a00c5962b73dfc169ce08f1cab9a on DrHyde:master.

coveralls commented 6 years ago

Coverage Status

Coverage increased (+0.0002%) to 99.967% when pulling 5e6d15195377fbdc5d71cfcc38ee2089c79a0fcb on dracos:79-national-formatting into 850ad3185171a00c5962b73dfc169ce08f1cab9a on DrHyde:master.

dracos commented 6 years ago

As with the other PR, I've reworked this to add a new formatter (and a helper function). Hope that's more in line with what you're after.

coveralls commented 6 years ago

Coverage Status

Coverage increased (+0.0005%) to 99.938% when pulling eaf938c33da0c0ce15f765adc287c53871e93150 on dracos:79-national-formatting into d281d6b5de5072565f4e91f232028740c64c9083 on DrHyde:master.

DrHyde commented 6 years ago

Thanks, this looks good. A few comments.

Do you think that the NationallyPreferred formatter should be renamed to something like NationallyPreferredIntl? It's a bit of a mouthful but I think it makes the difference between that and National a bit clearer.

Second, I'm not convinced that format_for_country is a good idea to expose to users or even that something like $ar_obj->format_for_country('GB') makes much sense. In general I think a user would care about the national number (so the new National formatter) and the international number (so NationallyPreferred(Intl) or E.123) but they won't care whether the person the international number is relevant to is in GB or RU or SS. And if the method is a good idea then I think it should be on the Number::Phone class, defaulting to returning E.123 if no country-specific formatting information is available.

Third, I wonder if there's some useful overlap with the dial_to method.

None of these are show-stoppers, but I'd like to at least discuss them before I merge and fiddle about with them.

dracos commented 6 years ago

Certainly happy to rename the formatter sure, I also couldn't think of anything nicer.

format_for_country, or something like it, is the main thing I actually want and is the reason for most/all of the other PRs :) It is definitely something I as a user want – I am running a website in one country (say the UK), people will be able to provide their mobile phone numbers, those mobile phone numbers won't all be UK ones, but as the site is UK-based I wish to display UK mobile phone numbers in their national UK format (07xxx...) and non-UK mobile phone numbers in their international format (e.g. +1...) – the display is always from the point of view of the site's location. This seems like it would be a common use case for displaying numbers to me. I don't mind where the method is.

dial_to certainly seems related, yes, and something a bit similar could be got by using a UK number as the from for the above – but I wouldn't want it being "clever" and not including the area code if it happened to match whatever I used for the 'from', if you see what I mean.

DrHyde commented 6 years ago

OK, that makes sense. Can you write a few lines of doco for format_for_country, then I'll get it merged, fiddle about a bit, and get it into the next release.

dracos commented 6 years ago

Done, thanks :)

DrHyde commented 6 years ago

Merged. I've done a bit of fiddling as discussed. I also made format_for_country and the two new formatters work even if the object being formatted doesn't have the requisite data, by just temporarily instantiating a stub object if necessary. It's a hack, but at least the functionality is available no matter how objects are created.