alcohol / iso3166

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

Create abstraction for the datasets #50

Closed xico42 closed 2 years ago

xico42 commented 4 years ago

Create the Dataset interface to encapsulate the operations with the country datasets. This should make extending the dataset easier and also make possible to decorate the data with additional information.

Some examples of applicability:

<?php

// Completely custom datasets
class DatabaseDataset implements Dataset {
    // ...
    public function lookup($key, $value) {
        $stmt = $this->pdo->prepare("select * from countries where :key = :value");
        $stmt->execute([':key' => $key, ':value' => $value]);
    }
}

// Decorating datasets
class LocalizedDataset implements Dataset {
    public function __construct(Dataset $wrapped) {
        $this->wrapped = $wrapped;
    }

    // ...

   public function lookup($key, $value) {
        $data = $this->wrapped->lookup($key, $value);
        $data['translated'] = \Locale::getDisplayRegion('-' . $data['alpha2'], 'pt_BR');
        return $data;
   }
}
codecov[bot] commented 4 years ago

Codecov Report

Merging #50 into master will not change coverage. The diff coverage is 100%.

Impacted file tree graph

@@           Coverage Diff           @@
##             master    #50   +/-   ##
=======================================
  Coverage       100%   100%           
- Complexity       33     42    +9     
=======================================
  Files             3      5    +2     
  Lines            75     95   +20     
=======================================
+ Hits             75     95   +20
Impacted Files Coverage Δ Complexity Δ
src/DefaultDataset.php 100% <100%> (ø) 1 <1> (?)
src/ISO3166.php 100% <100%> (ø) 16 <4> (ø) :arrow_down:
src/ArrayDataset.php 100% <100%> (ø) 8 <8> (?)

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 5283b39...3fe0a2a. Read the comment docs.

alcohol commented 4 years ago

I personally think it would be better if you encapsulate the ISO3166 class if you want to extend the dataset or decorate the data with additional information. That way you can write your own interface specific to your own needs, rather than trying to come up with something generic in this package.

xico42 commented 4 years ago

Yeah, it makes sense. After all, the ISO3166 class itself doesn't do much. However, it could still be interesting in case you wanted to address other ISO standards such as 3166-2/3, 4217, etc. It would make it easier to have different strategies regarding the way the dataset is read.

For example, the pycountry package acts as a wrapper over the data made available by debian. The pycountry package has a copy of the ISO data as json files in the repository itself.

Not sure if you have any plans towards making this kind of stuff possible in this package but in that case I would be happy to contribute.

alcohol commented 4 years ago

I have https://packagist.org/packages/alcohol/iso4217 also still floating around. I don't really have any plans for either. As it stands, my time is too limited to barely keep up with the open source projects I am involved in (aside from my own, I also spend a lot of time on Composer and other projects). I merely try to keep the data as up to date as possible based on my own research (sporadically) or user feedback/input.