Use java.util.Currency in currency related fakers

[panilya]

Issue: #1112

[what-the-diff]

PR Summary

• Enhancement of Country Details
  The Country details were enriched by adding information about available currencies in each country. This change allows us to access a random currency and its code associated with the specific country.

• De-emphasizing Currency Class
  The Currency class and its methods were marked as 'Deprecated', signifying plans for its removal in future versions. This change aims to encourage reduced reliance on this class.

• Amplifying Money Information
  Our money-related details were upgraded by including available currencies. This addition facilitates the retrieval of random currency details, its code, and numeric code.

• Removal of Redundant Details
  Currency-related entries were removed from the 'country.yml' file to avoid duplication since they are now directly accessible from the Country class.

• Deletion of Obsolete Files
  The 'money.yml' file was removed in line with the efforts to simplify our codebase and streamline information flow.

• Refinement of Testing Procedures
  Our testing procedures were revamped to accommodate these changes. Specifically, tests for currency and its code were removed from the 'CountryTest' class and added to the 'MoneyTest' class, in line with our shift in data organization.

[snuyanzin]

I tend to think that if we remove money.yml then we need to remove it from
net.datafaker.service.files.EnFile as well

[kingthorin]

Seems to be a lot of duplicate code, why not just have money and country call the methods in currency Money?

Also could use List.of instead of new ArrayList for the available currencies.

[snuyanzin]

Seems to be a lot of duplicate code, why not just have money and country call the methods in currency?
Also could use List.of instead of new ArrayList for the available currencies.

+1

Also how about making deprecated Currency extending Money and remove all the methods from Currency then it will automatically call methods from Money and no code duplication between these 2 classes ?

[bodiam]

Also could use List.of instead of new ArrayList for the available currencies.

-1. There's no such constructor to turn a Set into a List.

[bodiam]

Seems to be a lot of duplicate code, why not just have money and country call the methods in currency?

I'm not sure if something changed in the meantime, but there's hardly any code in Currency, which duplicate code do you mean?

[snuyanzin]

Also could use List.of instead of new ArrayList for the available currencies.

-1. There's no such constructor to turn a Set into a List.

in fact there is such, e.g. here

this.availableCurrencies = new ArrayList<>(Currency.getAvailableCurrencies());

and it could be replaced with (since we don't need mutable collection here it would be the advatage)

this.availableCurrencies = List.copyOf(Currency.getAvailableCurrencies());

May be duplicated code is not the right word here however e.g. Currency#code is

    public String name() {
        return resolve("currency.name");
    }

which does the same or almost the same thing as

public String currencyCode() {
        int randomIndex = faker.random().nextInt(availableCurrencies.size());
        return availableCurrencies.get(randomIndex).getCurrencyCode();
    }

[bodiam]

this.availableCurrencies = List.copyOf(Currency.getAvailableCurrencies());

copyOf would work, didn't think of that, nice! (not really sure how that would be much better than new ArrayList)

Regarding duplication

Sure, but that Currency thing is going to be deleted one day anyway, does it matter much? It's a genuine question, I'm just wondering if that should hold up this PR, that's all.

[snuyanzin]

Ok for Currency if it is going to be removed
I've just looked through the code once more and it looks like there is the duplicated logic for Money and for Country e.g.
Money#currencyCode and Country#currencyCode same for currency method. Why do we need this?

[panilya]

I've just looked through the code once more and it looks like there is the duplicated logic for Money and for Country e.g.
Money#currencyCode and Country#currencyCode same for currency method. Why do we need this?

I agree about duplicate code and will push the fix soon.

I thought about this (duplicate code or reuse Money faker) when I first implemented these changes, but I thought it might be better to not reuse Money (don't remember exactly the reason why I thought so, sorry). But it is better to reuse Money, indeed.

Thank you all for reviewing.

[bodiam]

Money#currencyCode and Country#currencyCode

Don't know, but we have these methods for years (one is from 2019, the other from 2022). And at least the currency codes are now gone from the YML, and these currency codes in money.yml and country.yml were only half duplicated. At least now they're in sync. Small bit of duplication; we could remove one of the methods, or have one of them call the other. Or we leave them like they are. I lean towards leaving them like they are, but if there's good arguments for a different approach, I'm fine with that too. Ideally I don't want to break the API too much, Currency being a bit of an exception perhaps.

[kingthorin]

RE: Duplication for my two cents these are basically all the same:

    public String currencyCode() {
        int randomIndex = faker.random().nextInt(availableCurrencies.size());
        return availableCurrencies.get(randomIndex).getCurrencyCode();
    }

They could be genericized to a single method that gets the index and accept a function to execute.

...or something like that. I haven't actually sat with an editor and tried/looked at it, that's just off the top of my head.

[kingthorin]

LGTM

[snuyanzin]

Thanks for addressing feedback