Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Specify standards where appropriate #315

Open
NikitaAvvakumov opened this issue Dec 2, 2019 · 2 comments
Open

Specify standards where appropriate #315

NikitaAvvakumov opened this issue Dec 2, 2019 · 2 comments

Comments

@NikitaAvvakumov
Copy link

Hi,

Thanks to all the contributors for their work on this resource!

I would like to suggest that it may be a good idea to specify which standards were used as source(s) of data in the library. For instance, in the English version of the Address module the country list contains "Svalbard & Jan Mayen Islands". However, ISO3166-1 defines this geographic entity as "Svalbard and Jan Mayen". There is also "Antarctica (the territory South of 60 deg S)", which is simply "Antarctica" as per ISO.

If the country names and other similar data in Faker originate from another source, it would be good to indicate this. This isn't just nitpicking - in one of our app's forms, a "Country" select is populated with ISO data, and tests ended up randomly failing when test schema were generated using Faker because e.g. "Svalbard & Jan Mayen Islands" was not a selectable option.

Thanks!

@igas
Copy link
Member

igas commented Dec 2, 2019

Hi @NikitaAvvakumov,

I agree, source will be a good to have. Speaking of your specific example, I feel like this would not be right to adjust it to your need, say we have someone else who want to use different list. My suggestion will be to add country/1, with default to :iso3166_1 and list from ISO. In this case you'll be able to use "correct" list in your app, but we leave place fore extensibility. we can also make it default for country/0. WDYT? PR would be much appreciated.

@NikitaAvvakumov
Copy link
Author

@igas Sounds good, thanks! I'll work on a PR. Do you know the source of the current country list so that it can be passed as an argument to country/1? In the interest of not breaking any current functionality, do you think it would be best to go with

defmodule Faker.Address.En do
  def country(data_source \\ :current)

  def country(:current) do
    # current list
  end

  def country(:iso3166_1) do
    # iso list
  end
end

Also, do you have any tips for testing this? I don't see a way to seed Faker's random number generator to make list sampling deterministic. Otherwise, I can only think of something indirect like

refute country(:iso3166_1) == "Svalbard & Jan Mayen Islands"
refute country(:current) == "Svalbard and Jan Mayen"

# OR

refute "Svalbard & Jan Mayen Islands" in Enum.map(1..big_number -> fn _ -> country(:iso3166_1) end)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants