-
Notifications
You must be signed in to change notification settings - Fork 557
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
Russian names support #14
Conversation
I appreciate the contribution, this looks very nice. I think the same sex stuff is a little complicated though. How do you feel about returning mixed names (male/female) on Faker::NameRU.first_name / Faker::NameRU.last_name and adding additional methods to return either male or female names?
Faker returns both male and female names for the other name modules so that would follow the "convetion" (de facto convention really :-). Another (perhaps more clean) option to simplify the code and make it work as the other modules would be to add an optional param defaulting to mixed mode:
|
I think we misunderstood each other a little :) Currently methods like #last_name and #first_name already return mixed sex unless they are called from a block passed to #with_same_sex like this: And what I originally wanted to say is that #name returns a full name collected from last_name, first_name and patronymic which are all of the same sex (internally it uses #with_same_sex, which picks a random sex just before yielding and doesn't let the code in the block get a new one). I also thought about adding a sex param to all methods but i'm not sure it would be useful - usually sex should also be random. |
While working on one project yesturday I encountered a situation where I really need to generate exactly male name, so I'm taking my words back :) I've added an optional sex parameter to all methods. |
Gotcha. Ok, I pulled your branch and did a small change, I removed the "with_same_sex" method. I think it is pretty easy to have all the names in the same sex passing always the same sex to faker, and it reduces a lot the module size. if users wants the same sex for all their names always, they can always use a variable like this:
etc.. Does that look ok? |
All right. I restored the with_same_sex method but refactored it a bit too. Please take a look and tell me if it looks good, so I can merge it and release it. |
Ok, it looks a lot better, thanks, but there's a little bug inside #name: it has to generate all of it's components with the same sex (and therefore test_name_sex failed from time to time), so I restored a call to #with_same_sex from the #name. Feel free to refactor it if you don't like the syntax, everything else seems fine. |
Merged and released. Thanks for your contribution! |
Hello.
I've added a Faker::NameRU module which generates russian names. In russian last name depends on gender, so both last name, first name and patronymic have to be of same gender; therefore I had to implement separate name lists for males and females. Hope this doesn't break the ffaker philosophy.
Please pull or notify me to change something if it doesn't fit.