rinvex / countries

Rinvex Country is a simple and lightweight package for retrieving country details with flexibility. A whole bunch of data including name, demonym, capital, iso codes, dialling codes, geo data, currencies, flags, emoji, and other attributes for all 250 countries worldwide at your fingertips.
https://rinvex.com
MIT License
1.65k stars 206 forks source link

Fix longlist for currencies #167

Closed fritzmg closed 2 years ago

fritzmg commented 4 years ago

Currently, if you execute

$currencies = currencies(true);

or

$currencies = \Rinvex\Country\CurrencyLoader::currencies(true);

the resulting array will be empty.

The reason is the filtering:

https://github.com/rinvex/countries/blob/c6f08bdb66e37a01007213c5e88f6a7a1fa33933/src/CurrencyLoader.php#L38-L44

This only works for the shortlist, but not the longlist. And even if the filtering was removed, the subsequent sort() would not work (since it would need to be ksort in that case) and the array_merge would destroy the longlist too.

This PR actually changes a few more things:

  1. It implements an early out with the static variable.
  2. It only uses ksort for the longlist (no filtering should be necessary, as the associative array by ISO code is unique already).
  3. It stores the final result for the shortlist and longlist in the static variable, including filtering and sorting.
fritzmg commented 4 years ago

I don't really undesrtand the error from codeclimate:

Method currencies has 26 lines of code (exceeds 25 allowed). Consider refactoring.

Why is it only allowed to have 25 lines of code?

codeclimate[bot] commented 4 years ago

Code Climate has analyzed commit 742e5c5d and detected 1 issue on this pull request.

Here's the issue category breakdown:

Category Count
Complexity 1

View more on Code Climate.

fritzmg commented 4 years ago

I made some further simplifications to the code. Also I changed the test, reflecting that the longlist actually returns more currencies than the shortlist. These are the currencies that the longlist returns additionally:

Omranic commented 2 years ago

Thank you for your patience, this is now fixed in https://github.com/rinvex/countries/commit/1475ba9c252bc7902268f2a9551a47bd2af54f2b