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

Method to get any random animal #1618

Closed
kharann opened this issue Nov 28, 2022 · 21 comments · Fixed by #2786
Closed

Method to get any random animal #1618

kharann opened this issue Nov 28, 2022 · 21 comments · Fixed by #2786
Labels
c: feature Request for new feature m: animal Something is referring to the animal module s: waiting for user interest Waiting for more users interested in this feature
Milestone

Comments

@kharann
Copy link

kharann commented Nov 28, 2022

Clear and concise description of the problem

Hi! I just want a method that returns an animal, of any type. Sometimes you just want any kind of animal rather than one of a specific species.

Suggested solution

maybe something like

faker.animal.animal()
// or
faker.animal.random()

Alternative

No response

Additional context

No response

@kharann kharann added the s: pending triage Pending Triage label Nov 28, 2022
@ejcheng ejcheng added c: feature Request for new feature m: animal Something is referring to the animal module s: needs decision Needs team/maintainer decision and removed s: pending triage Pending Triage labels Nov 29, 2022
@ejcheng
Copy link
Member

ejcheng commented Nov 29, 2022

Perhaps we should have a new function in the animal module that pulls from all animal definitions?

I'm pretty sure we shouldn't use random as a method name because all methods return random data.

@ST-DDT
Copy link
Member

ST-DDT commented Nov 29, 2022

Instead of returning a complex breed or race of animal, the new method should return the type or category (e.g. dog, cat, ...). Not sure what the actual name for that is. Maybe just type()?

@Shinigami92
Copy link
Member

We could still stick to what we discussed for other stuff and just use sample

To shorten it for the long run, we could also use sample({ category/species?: '' })

Beside that type is not what the issue creator asked for, I'm against type as it conflicts with TypeScript's keyword type
That is why many code bases written in TS uses kind instead

@ST-DDT
Copy link
Member

ST-DDT commented Nov 29, 2022

We could still stick to what we discussed for other stuff and just use sample

I also thought about that but I would expect sample to return something like a mixture of the others e.g. Messouri Fox Trotter (Horse) or Red House Cat.

Beside that type is not what the issue creator asked for, I'm against type as it conflicts with TypeScript's keyword type
That is why many code bases written in TS uses kind instead

I think type/category/kind (e.g. cat, lion, eagle) is exactly what @kharann asked for (Please correct me if I'm wrong). I fine with naming it kind over type. But I would have to spend some time to browse some animal categorization literature and their simplified variants.

@kharann
Copy link
Author

kharann commented Nov 29, 2022

We could still stick to what we discussed for other stuff and just use sample

I also thought about that but I would expect sample to return something like a mixture of the others e.g. Messouri Fox Trotter (Horse) or Red House Cat.

Beside that type is not what the issue creator asked for, I'm against type as it conflicts with TypeScript's keyword type
That is why many code bases written in TS uses kind instead

I think type/category/kind (e.g. cat, lion, eagle) is exactly what @kharann asked for (Please correct me if I'm wrong). I fine with naming it kind over type. But I would have to spend some time to browse some animal categorization literature and their simplified variants.

I did ask for any kind of animal. Right now you have to choose a category, as there's no method that returns any kind of animal

@Shinigami92
Copy link
Member

Oh thanks for the clarification, so I really misunderstood you first, I'm sorry 🙇

So yeah, we could go with kind or category
A brain-cell in my head yells that species is not correct here 🤔

@kharann
Copy link
Author

kharann commented Nov 29, 2022

Honestly, at first i was thinking something like faker.animal.any(), but I think any is also kinda cursed as a word in TypeScript world

@Shinigami92
Copy link
Member

Honestly, at first i was thinking something like faker.animal.any(), but I think any is also kinda cursed as a word in TypeScript world

We had this discussion already with the word module.
Yes we want to omit terms like type, any and random

@matthewmayer matthewmayer linked a pull request Feb 24, 2023 that will close this issue
@xDivisionByZerox xDivisionByZerox added s: waiting for user interest Waiting for more users interested in this feature and removed s: needs decision Needs team/maintainer decision labels Mar 9, 2023
@xDivisionByZerox
Copy link
Member

Team decision

If you want/need this feature, please upvote this issue.

You can use the following as a workaround in the meantime.

faker.helpers.objectValue(faker.animal)();

@Shinigami92
Copy link
Member

Shinigami92 commented Mar 10, 2023

Team decision

If you want/need this feature, please upvote this issue.

You can use the following as a workaround in the meantime.

faker.helpers.objectValue(faker.animal)();

This is for now an okay-ish workaround, but keep in mind that it is not future proof to do this, as well as it theoretically comes with a "bug" on the expectation point of view.

  1. When a new method will be introduces in the animal module that is unrelated and do something new, it will get called additionally without the knowledge of the dev that implemented this workaround
  2. Already right now: in a chance of 1 to 15 the type method would get called which does not return an animal but the type of an animal
    But theoretically this is not that important if it only is used for a test, as then it is "just" a string. But keep in mind if you doing it for Demo purposes.
    So maybe you might then need to filter out the type method (the current addressing PR also has this bug and would need to be adjusted) nevermind, it uses a static selection of typeOfAnimals

@matthewmayer
Copy link
Contributor

Would it be good to have a method which just returns a "simple" animal? Like i might just want a common one word animal name like:

dog
cat
monkey
elephant
deer
camel
whale
sheep
goat

rather than African Slender-snouted Crocodile

@ST-DDT
Copy link
Member

ST-DDT commented Mar 10, 2023

Would it be good to have a method which just returns a "simple" animal? Like i might just want a common one word animal name like:

dog
cat
monkey
elephant
deer
camel
whale
sheep
goat

rather than African Slender-snouted Crocodile

https://fakerjs.dev/api/animal.html#type

@ST-DDT ST-DDT added this to the vFuture milestone Mar 10, 2023
@matthewmayer
Copy link
Contributor

Is that localizable though?

@ST-DDT
Copy link
Member

ST-DDT commented Mar 10, 2023

Is that localizable though?

type(): string {
return this.faker.helpers.arrayElement(this.faker.definitions.animal.type);
}

Sure.

@matthewmayer
Copy link
Contributor

matthewmayer commented Mar 10, 2023

hmm but it looks like its supposed to return one of the other method names, ie you can do:

faker.animal[faker.animal.type()]() to get a animal of any type? Or is that a coincidence and not expected to be maintained? i think "crocodilia", "cetacean" etc are not what i'd expect if i wanted an animal?

@Shinigami92
Copy link
Member

Seems like in v5.5.3 they were already localized (seems wired to me and like a design issue)
But at least the en locale entries reflect exact what were used as method names (as my expectations)
So at the end, I have the same expectations as @matthewmayer for the type function
Maybe we could even convert this into an Enum like Sex/SexType and the others

@ST-DDT
Copy link
Member

ST-DDT commented Mar 10, 2023

I'm not sure whether cetacean or crocodilia is the correct return value either, I think crocodile is fine though.
I'm also not sure whether type() is the correct name for the method.
But IMO that method should definitely not return Object.keys(faker.animal) or a subset thereof - or if it does we should have one that does not.

@matthewmayer
Copy link
Contributor

Would you consider adding additional animals to the definitions.animal.type which don't correspond to method names to be a breaking change?

@ST-DDT
Copy link
Member

ST-DDT commented Mar 12, 2023

Would you consider adding additional animals to the definitions.animal.type which don't correspond to method names to be a breaking change?

No, neither the jsdocs nor the implementation hint at it being a key of callable methods.

Also FR already has its own translations:
https://github.com/faker-js/faker/blob/next/src/locales/fr/animal/type.ts

@matthewmayer
Copy link
Contributor

Then I think we could just add a bunch more simple animals to this method? It's common to use animals as placeholder names eg on google docs

https://www.quora.com/What-is-the-complete-list-of-anonymous-creatures-one-can-be-labeled-as-when-using-Google-Docs

@ST-DDT ST-DDT added s: waiting for user interest Waiting for more users interested in this feature and removed s: waiting for user interest Waiting for more users interested in this feature labels May 5, 2023
@github-actions
Copy link
Contributor

github-actions bot commented May 5, 2023

Thank you for your feature proposal.

We marked it as "waiting for user interest" for now to gather some feedback from our community:

  • If you would like to see this feature be implemented, please react to the description with an up-vote (:+1:).
  • If you have a suggestion or want to point out some special cases that need to be considered, please leave a comment, so we are aware about them.

We would also like to hear about other community members' use cases for the feature to give us a better understanding of their potential implicit or explicit requirements.

We will start the implementation based on:

  • the number of votes (:+1:) and comments
  • the relevance for the ecosystem
  • availability of alternatives and workarounds
  • and the complexity of the requested feature

We do this because:

  • There are plenty of languages/countries out there and we would like to ensure that every method can cover all or almost all of them.
  • Every feature we add to faker has "costs" associated to it:
    • initial costs: design, implementation, reviews, documentation
    • running costs: awareness of the feature itself, more complex module structure, increased bundle size, more work during refactors

View more issues which are waiting for user interest

matthewmayer added a commit to matthewmayer/faker that referenced this issue Apr 5, 2024
fix faker-js#1618

This adds more common animals and uses more common names for existing ones e.g. cetecean->whale, crocodilia->crocodile
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c: feature Request for new feature m: animal Something is referring to the animal module s: waiting for user interest Waiting for more users interested in this feature
Projects
None yet
6 participants