alcohol / iso3166

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

Adding a common exception interface #53

Closed Einenlum closed 4 years ago

Einenlum commented 4 years ago

First of all, thanks for this great library :).

Right now, if I want to check if a country code is valid, I need to do this:

try {
    (new ISO3166())->alpha2($value);

    return true;
} catch (InvalidArgumentException $e) {
} catch (OutOfBoundsException $e) {
} catch (DomainException $e) {
}

return false;

or (for PHP ^7.1):

try {
    (new ISO3166())->alpha2($value);

    return true;
} catch (InvalidArgumentException | OutOfBoundsException | DomainException $e) {
    return false;
}

This pull request proposes to add a common exception interface for these three exceptions. Therefore I don't have to check first if my $value argument is a string that matches 2 chars. If there is an ISO3166Exception, and I don't need more details, I know right away that the value I gave leads to nowhere.

New possible behavior (with no BC break):

try {
    (new ISO3166())->alpha2($value);

    return true;
} catch (\League\ISO3166\Exception\ISO3166Exception $e) {
    return false;
}
codecov[bot] commented 4 years ago

Codecov Report

Merging #53 into master will not change coverage. The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##             master    #53   +/-   ##
=======================================
  Coverage       100%   100%           
  Complexity       33     33           
=======================================
  Files             3      3           
  Lines            75     75           
=======================================
  Hits             75     75

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 86f9972...dc18f9a. Read the comment docs.

alcohol commented 4 years ago

While you could achieve the same by simply using catch (Exception $e), I can see the value for a namespaced interface. I would however like to propose the new interface extends from \Throwable. What are your thoughts on this?

alcohol commented 4 years ago

Also sorry for not replying sooner, it seems this repository was no longer on my watched list after I did a perhaps too rigorous cleanup of said list.

Einenlum commented 4 years ago

I would however like to propose the new interface extends from \Throwable. What are your thoughts on this?

Oh, clever! :+1:

Also sorry for not replying sooner, it seems this repository was no longer on my watched list after I did a perhaps too rigorous cleanup of said list.

No worries, it's definitely not an urgent "issue" ^^. And less than 2 weeks for an answer in FOSS is clearly a good metric :) Thanks for your answer.

alcohol commented 4 years ago

When you can find the time to add the extends from, I'll gladly merge this and tag a new release :-)

Einenlum commented 4 years ago

@alcohol Done :)

alcohol commented 4 years ago

Cheers https://github.com/thephpleague/iso3166/releases/tag/2.1.5