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

Add the concept of order for firstName and lastName in name.fullName #1211

Closed
satoc0629 opened this issue Jul 30, 2022 · 11 comments · Fixed by #1637
Closed

Add the concept of order for firstName and lastName in name.fullName #1211

satoc0629 opened this issue Jul 30, 2022 · 11 comments · Fixed by #1637
Assignees
Labels
c: feature Request for new feature c: locale Permutes locale definitions m: person Something is referring to the person module p: 1-normal Nothing urgent s: accepted Accepted feature / Confirmed bug
Milestone

Comments

@satoc0629
Copy link
Contributor

Clear and concise description of the problem

The sequence of Japanese names is lastName followed by firstName in the Japanese-speaking world.
ex: YAMADA TARO.
YAMADA is lastName.
TARO is firstName.
I would like to incorporate this into faker as well.

// This sentence relies on machine translation.

Suggested solution

Add full_name_order to NameDefinitions.
The full_name_order is as follows.

export type FullNameOrder = 'normal' | 'reverse';

Add the ordering process using this to fullName in src/modules/name/index.ts.

    if (this.faker.definitions.name.full_name_order === 'reverse') {
      nameParts.push(lastName);
      nameParts.push(firstName);
    } else {
      nameParts.push(firstName);
      nameParts.push(lastName);
    }

Alternative

I do not understand Faker or programming that deeply, so the best approach may be something else.

Additional context

In addition to this, we would like to add a collection of first names for Japanese men and a collection of first names for Japanese women.

@satoc0629 satoc0629 added the s: pending triage Pending Triage label Jul 30, 2022
@xDivisionByZerox xDivisionByZerox added c: feature Request for new feature p: 1-normal Nothing urgent s: needs decision Needs team/maintainer decision c: locale Permutes locale definitions m: person Something is referring to the person module and removed s: pending triage Pending Triage labels Jul 30, 2022
@xDivisionByZerox
Copy link
Member

xDivisionByZerox commented Jul 30, 2022

The suggested solution you provided sounds reasonable to me. Other locales could gain value from this as well.

In my opinion, we should call the property in the definition default_full_name_order. Additionally, I would suggest adding an option to name.fullName() that allows overriding of the default behavior for the current locale:

// en version, but that doesn't matter
import { faker } from '@faker-js/faker';

console.log(faker.definitions.name.full_name_order); // 'normal'

// call with no params resolves to the current value of the locale definition (in this case 'normal')
console.log(faker.name.fullName()); // Lexie Bernie

// explicitly set the order, but this doesn't do anything in this case as it's the same as the definitions
console.log(faker.name.fullName({ order: 'normal' })); // Lexie Bernie

// explicitly set the order and override the default value
console.log(faker.name.fullName({ order: 'reverse' })); // Bernie Lexie

@xDivisionByZerox xDivisionByZerox changed the title Added the concept of the order of first and last name in FullName. Add the concept of order for firstName and lastName in name.fullName Jul 30, 2022
@ejcheng
Copy link
Member

ejcheng commented Jul 30, 2022

The suggested solution you provided sounds reasonable to me. Other locales could gain value from this as well.

In my opinion, we should call the property in the definition default_full_name_order. Additionally, I would suggest adding an option to name.fullName() that allows overriding of the default behavior for the current locale:

// en version, but that doesn't matter
import { faker } from '@faker-js/faker';

console.log(faker.definitions.name.full_name_order); // 'normal'

// call with no params resolves to the current value of the locale definition (in this case 'normal')
console.log(faker.name.fullName()); // Lexie Bernie

// explicitly set the order, but this doesn't do anything in this case as it's the same as the definitions
console.log(faker.name.fullName({ order: 'normal' })); // Lexie Bernie

// explicitly set the order and override the default value
console.log(faker.name.fullName({ order: 'reverse' })); // Bernie Lexie

Yep, Chinese is also "lastname firstname".

@Shinigami92
Copy link
Member

I like the idea, but I wont name it normal and reversed as this is just "our" "western" normal, but in asian countries the "normal" is our "reversed" 🤔

@ST-DDT
Copy link
Member

ST-DDT commented Jul 30, 2022

Hasn't Spanish(?) some entirely different naming scheme?
E.g. The full name consists also of the parents name?

Currently in Spain, people bear a single or composite given name (nombre in Spanish) and two surnames (apellidos in Spanish).

Source: https://en.wikipedia.org/wiki/Spanish_naming_customs

@Minozzzi
Copy link
Contributor

Maybe we can use parameters by country name liked this

import { faker } from '@faker-js/faker';

console.log(faker.name.fullName()); // Lexie Bernie

console.log(faker.name.fullName({ country: 'Japan' })); // Bernie Lexie

@xDivisionByZerox
Copy link
Member

xDivisionByZerox commented Jul 30, 2022

Maybe we can use parameters by country name liked this

import { faker } from '@faker-js/faker';

console.log(faker.name.fullName()); // Lexie Bernie

console.log(faker.name.fullName({ country: 'Japan' })); // Bernie Lexie

If you want Japanese you are most likely using the jp locale. In that case, you expect all faker results to return values that align with the Japanese language.

import { faker } from '@faker-js/faker';
faker.setLocale('jp'); // I'm expecting faker to return Japanese values across all modules

// your suggestion
console.log(faker.name.fullName({ country: 'Japan' }));
// why would I need to set the country to 'Japan' here? I'm already using the Japanese locale.

Your suggestion has another flaw. If you provide a country you would have to support all (or most) of the 195 countries there are and most of them would have overlapped for the "format". This would lead to really hard-to-maintain code because of the introduced redundancy.

@Minozzzi
Copy link
Contributor

Talvez possamos usar parâmetros por nome de país como este

import { faker } from '@faker-js/faker';

console.log(faker.name.fullName()); // Lexie Bernie

console.log(faker.name.fullName({ country: 'Japan' })); // Bernie Lexie

Se você quiser japonês, provavelmente está usando a jplocalidade. Nesse caso, você espera que todos os resultados falsos retornem valores alinhados ao idioma japonês.

import { faker } from '@faker-js/faker';
faker.setLocale('jp'); // I'm expecting faker to return Japanese values across all modules

// your suggestion
console.log(faker.name.fullName({ country: 'Japan' }));
// why would I need to set the country to 'Japan' here? I'm already using the Japanese locale.

Sua sugestão tem outra falha. Se você fornecer um country, terá que apoiar todos (ou a maioria) dos 195 países que existem e a maioria deles teria sobreposto para o "formato". Isso levaria a um código realmente difícil de manter por causa da redundância introduzida.

That makes sense.

@satoc0629
Copy link
Contributor Author

Thank you all for your input.
I have considered the following.
First, I thought that this issue is a cultural trait rooted in some countries and should not be generalized as a parameter.
I think FullName in each locale should be possible during the localization process. I think this can be enumerated as full_name_pattern.ts.
That is, for locales where this is configured, it would be honored, and if not configured, it would provide the same behavior as before.
I have expressed my concept in a Pull Request.

Translated with www.DeepL.com/Translator (free version)

@satoc0629
Copy link
Contributor Author

We are changing our policy to take advantage of legacy templates as you have indicated.

And we are considering the following modifications to the fullName function.

  fullName(
    options: {
      firstName?: string;
      lastName?: string;
      gender?: GenderType;
    } = {}
  ): string {
    const {
      gender = this.faker.helpers.arrayElement(['female', 'male']),
      firstName = this.firstName(gender),
      lastName = this.lastName(gender),
    } = options;

    const prefix = this.faker.helpers.maybe(() => this.prefix(gender), {
      probability: 0.125,
    });

    const suffix = this.faker.helpers.maybe(() => this.suffix(), {
      probability: 0.125,
    });

    const nameParts = [];
    if (prefix) {
      nameParts.push(prefix);
    }
    nameParts.push(firstName);
    nameParts.push(lastName);
    if (suffix) {
      nameParts.push(suffix);
    }
    const defaultName = nameParts.join(' ');

    if (
      !this.faker.definitions.name.name ||
      this.faker.definitions.name.name.length === 0
    ) {
      return defaultName;
    }
    // if (gender || firstName || lastName) {
    //   return defaultName;
    // }

    const fullNamePattern = this.faker.helpers.arrayElement(
      this.faker.definitions.name.name
    );

    const fullName = this.faker.helpers.mustache(fullNamePattern, {
      'name.gender': () => this.gender(),
      'name.binary_gender': () => this.gender(true),
      'name.prefix': () => this.prefix(gender),
      'name.female_prefix': () => this.prefix(gender),
      'name.male_prefix': () => this.prefix(gender),
      'name.first_name': () => (firstName ? firstName : this.firstName(gender)),
      'name.female_first_name': () =>
        firstName ? firstName : this.firstName(gender ? gender : 'female'),
      'name.male_first_name': () =>
        firstName ? firstName : this.firstName(gender ? gender : 'male'),
      'name.middle_name': () => this.middleName(gender),
      'name.female_middle_name': () =>
        this.middleName(gender ? gender : 'female'),
      'name.male_middle_name': () => this.middleName(gender ? gender : 'male'),
      'name.last_name': () => (lastName ? lastName : this.lastName(gender)),
      'name.female_last_name': () =>
        lastName ? lastName : this.lastName(gender ? gender : 'female'),
      'name.male_last_name': () =>
        lastName ? lastName : this.lastName(gender ? gender : 'male'),
      'name.suffix': () => (suffix ? suffix : this.suffix()),
    });

    return fullName;
  }

Retrieve the template from the legacy NameDefinitions.name and convert it.

Do you have a better idea about this?
I have done my best to write it, but I feel something is missing.

Thanks for reading.

@ST-DDT ST-DDT linked a pull request Aug 4, 2022 that will close this issue
@matthewmayer
Copy link
Contributor

Just to note, there's not always a space between the "last" (family) name and first name.

For example in zh-CN you'd want "{{last_name}}{{first_name}}"

@ST-DDT
Copy link
Member

ST-DDT commented Jan 26, 2023

Team decision

We want the full name patterns.

@ST-DDT ST-DDT added s: accepted Accepted feature / Confirmed bug and removed s: needs decision Needs team/maintainer decision labels Jan 26, 2023
@ST-DDT ST-DDT added this to the v8.0 - Module Re-Shuffling milestone Jan 26, 2023
@ST-DDT ST-DDT moved this to In Progress in Faker Roadmap Jan 26, 2023
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 c: locale Permutes locale definitions m: person Something is referring to the person module p: 1-normal Nothing urgent s: accepted Accepted feature / Confirmed bug
Projects
No open projects
Status: Done
7 participants