Add lastName gender specific on ru_RU locale #1747
Conversation
|
Thank you! Just faced same issue and wanted to add a pull request. |
Optimize code Co-Authored-By: Andreas Möller <am@localheinz.com>
|
@aanfarhan @antonkomarev my russian is not so good but is this valid for all male names that they have a female equivalent with an appended "a"? |
| { | ||
| $lastName = static::randomElement(static::$lastName); | ||
|
|
||
| if (static::GENDER_FEMALE === $gender) { |
There was a problem hiding this comment.
Maybe we should add a link here for explanation?
For example, https://en.m.wikipedia.org/wiki/Eastern_Slavic_naming_customs#Grammar.
/cc @pimjansen
There was a problem hiding this comment.
Agree, im not a fan of "only in our usecases" but it will do for no i guess
src/Faker/Provider/ru_RU/Person.php
Outdated
| /** | ||
| * Return last name for the specified gender. | ||
| * | ||
| * @access public |
There was a problem hiding this comment.
Not sure if using the @access annotation here is something we really need - it duplicates the public visibility keyword that is already present. Can we remove it?
|
@pimjansen not all the female last names ends with |
| * for. If the argument is skipped a random gender will be used. | ||
| * @return string Last name | ||
| */ | ||
| public function lastName($gender = null) |
There was a problem hiding this comment.
Do I understand correctly or is the generated lastName always Male by default? Shouldn't it return a random gender?
There was a problem hiding this comment.
There was a problem hiding this comment.
Alright, I'll add some adjustment to the code.
| */ | ||
| private $faker; | ||
|
|
||
| public function setUp() |
There was a problem hiding this comment.
Visibility should be protected here, but we can - with the right rules - automatically fix.
| return $lastName; | ||
| } | ||
|
|
||
| return $lastName . static::randomElement(static::$lastNameSuffix); |
There was a problem hiding this comment.
What do you think about
| return $lastName . static::randomElement(static::$lastNameSuffix); | |
| return static::lastName(static::randomElement(array( | |
| static::GENDER_FEMALE, | |
| static::GENDER_MALE, | |
| )); |
?
Then we can also do away with the newly introduced protected property.
src/Faker/Provider/ru_RU/Person.php
Outdated
| /** | ||
| * Return last name for the specified gender. | ||
| * | ||
| * @access public |
src/Faker/Provider/ru_RU/Person.php
Outdated
| return $lastName; | ||
| } | ||
|
|
||
| return static::lastName(static::randomElement(array( |
There was a problem hiding this comment.
this won't work: Person::lastName() isn't a static function. Also, I'm bot a big fan of using recursion here - I think it's overkill. Can you revert you last commit? Or perhaps you meant Person::$lastName?
|
Thanks! |
Add
lastNamemethod on Person Provider ru_RU locale with gender params.Related to #1745.