alcohol / iso3166

A PHP library providing ISO 3166-1 data.
https://iso3166.thephpleague.com
MIT License
640 stars 59 forks source link

Throw an exception when multiple country codes are retrieved #94

Closed tomSasaki closed 1 year ago

tomSasaki commented 1 year ago

Hello! Thank you for creating such a helpful library.

I've noticed something since we can now do a partial match search for country names.

【What I've noticed】 $data = (new League\ISO3166\ISO3166)->name("Bah");

I was trying to find a country name that includes 'Bah'. However, the result was 'Bahamas' instead of 'Bahrain'.

【Suggestion】 How about making it so that an error occurs when two or more country names are found with a partial match search?

I've written a patch and test code for this. I'd appreciate it if you could check it.

Thank you as always.

alcohol commented 1 year ago

I see how this could be an issue depending on your expectations.

Right now it simply returns the very first match found. There is no clear indication there could be more than one match (undocumented). I am not sure what the desired behavior would be though.


Throwing an exception

This way you immediately know there is more than one result. But you don't know if it is limited to two or possibly more. Also, you don't get any results back at all this way. Personally I think this is the least preferable solution.

:boom: This is a breaking change requiring a major version bump.


Return a multidimensional array

We could return all matches found in an array. But since I decided not to use value objects so far, it could be a bit awkward to check for whether or not this is the case (from a user point of view).

:boom: This is a breaking change requiring a major version bump.


Change nothing but document behavior

We could document the method to elaborate that it simply returns the first matching value. Downside is that most users probably don't read that stuff.

:wrench: This merely requires a bump in patch version.

tomSasaki commented 1 year ago

Hi @alcohol,

Thank you for your detailed response and for considering my proposal. I understand your concerns about throwing an exception and the potential impact it could have on the library.

In light of your comments, I'd like to propose an alternative. Would it be possible to provide a method for exact match searches? This method could function similarly to the 'name' method that was available before version 4.0.0.

This solution would enable exact match searches without impacting the current partial match functionality, and it wouldn't necessitate a major version bump. I believe this would be a valuable addition to the library.

Thank you for your time and consideration. I appreciate your work on this library and am looking forward to future improvements.

Best Regards,

alcohol commented 1 year ago

I am definitely open to that suggestion. If you would submit a PR, we can work out the little details along the way.

tomSasaki commented 1 year ago

Thank you for your acceptance of the proposal. I'll proceed to submit a PR. Best regards.