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

feat::Add multilanguage sexes (Issue #251) #312

Merged
merged 4 commits into from
Dec 10, 2023

Conversation

qmohitsingh
Copy link
Contributor

@qmohitsingh qmohitsingh commented Nov 22, 2023

Title: Add Multilingual Support for Sex Descriptions

Key Changes:

  • A new map sexTranslations is added, which maps each Language enum to another map. This inner map pairs the Sex enum (Male, Female) with their respective translations in that language.
  • The translateSex function is implemented to fetch these translations. It uses the Language and Sex enums to return the correct localized string.
  • The Person class's sex() method is updated to include an optional Language parameter, defaulting to English. This change allows users to retrieve the sex description in the desired language.

Example Usage:

  • In Poland: Person::sex(Language::Polish) would return "Mężczyzna" for Male and "Kobieta" for Female.

@qmohitsingh
Copy link
Contributor Author

hey @cieslarmichal, could you please review this PR and let me know if this is what expected or i need to make any sort of modifications? Thank you

Copy link
Owner

@cieslarmichal cieslarmichal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let me know when it will be done and ready to merge as I see it is now WIP.

include/faker-cxx/types/Sex.h Show resolved Hide resolved
include/faker-cxx/types/Sex.h Show resolved Hide resolved
include/faker-cxx/types/Sex.h Show resolved Hide resolved
@cieslarmichal
Copy link
Owner

hey, any updates? :)

@qmohitsingh
Copy link
Contributor Author

hey, any updates? :)

Hey, I've been digging through my Git history, trying to find the commit where you removed the Language enum, but I'm hitting a wall. I used git log --grep="remove Language enum" to search for it, but no luck. Any tips or tricks you can share to help me locate that commit? I'd really appreciate it!

@cieslarmichal
Copy link
Owner

Ok, sorry I should have give you better instructions about that Language enum, here it is:

#pragma once

#include <map>
#include <vector>

namespace faker
{
enum class Language
{
    English,
    Polish,
    Italian,
    French,
    German,
    Russian,
    Romanian,
    Hindi,
    Finnish,
    Nepali,
    Spanish,
    Turkish,
    Czech,
    Slovak,
    Ukrainian,
    Danish,
    Swedish
};

const std::vector<Language> languages{
    Language::English,  Language::Polish, Language::Italian,   Language::French, Language::German,  Language::Russian,
    Language::Romanian, Language::Hindi,  Language::Finnish,   Language::Nepali, Language::Spanish, Language::Turkish,
    Language::Czech,    Language::Slovak, Language::Ukrainian, Language::Danish, Language::Swedish};

inline std::string toString(Language language)
{
    std::map<Language, std::string> languageToStringMapping{
        {Language::English, "English"},   {Language::Polish, "Polish"},   {Language::Italian, "Italian"},
        {Language::French, "French"},     {Language::German, "German"},   {Language::Russian, "Russian"},
        {Language::Romanian, "Romanian"}, {Language::Hindi, "Hindi"},     {Language::Finnish, "Finnish"},
        {Language::Nepali, "Nepali"},     {Language::Spanish, "Spanish"}, {Language::Turkish, "Turkish"},
        {Language::Czech, "Czech"},       {Language::Slovak, "Slovak"},   {Language::Ukrainian, "Ukrainian"},
        {Language::Danish, "Danish"},     {Language::Swedish, "Swedish"}};

    return languageToStringMapping.at(language);
}

inline std::ostream& operator<<(std::ostream& os, Language language)
{
    return os << toString(language);
}

}

It was in include/faker-cxx/types/Language.h

@cieslarmichal
Copy link
Owner

It would be nice to have all corresponding languages to countries we have in types/Country.h

@qmohitsingh
Copy link
Contributor Author

It would be nice to have all corresponding languages to countries we have in types/Country.h

gotcha! i will make that change, thank you

@codecov-commenter
Copy link

codecov-commenter commented Nov 24, 2023

Codecov Report

Attention: 29 lines in your changes are missing coverage. Please review.

Comparison is base (0bbc251) 98.82% compared to head (9e9d902) 98.04%.

Files Patch % Lines
include/faker-cxx/types/Language.h 0.00% 19 Missing ⚠️
include/faker-cxx/types/Sex.h 0.00% 10 Missing ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #312      +/-   ##
==========================================
- Coverage   98.82%   98.04%   -0.79%     
==========================================
  Files          47       48       +1     
  Lines        3143     3167      +24     
==========================================
- Hits         3106     3105       -1     
- Misses         37       62      +25     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Estonian
};

//const std::vector<Language> languages{
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please remove commented code

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed

include/faker-cxx/types/Sex.h Show resolved Hide resolved
include/faker-cxx/Person.h Show resolved Hide resolved
@qmohitsingh qmohitsingh changed the title [WIP] feat::Add multilanguage sexes (Issue #251) feat::Add multilanguage sexes (Issue #251) Nov 24, 2023
@cieslarmichal
Copy link
Owner

any progress?

@qmohitsingh
Copy link
Contributor Author

any progress?

Unfortunately, I've been tied up with other work, so I haven't had the chance to work on this yet. But I'm planning to get it done this weekend. I hope that's okay

@cieslarmichal
Copy link
Owner

Ok sure, no problem :)

@cieslarmichal
Copy link
Owner

hello, are you going to finish this PR?

@cieslarmichal
Copy link
Owner

I will close the PR after 3 days from now if not updated.

@qmohitsingh
Copy link
Contributor Author

I will close the PR after 3 days from now if not updated.

I should be able to finish it today, i just need to write some test cases for it now

@cieslarmichal
Copy link
Owner

Good job! :D

@cieslarmichal cieslarmichal merged commit 4ec41f9 into cieslarmichal:main Dec 10, 2023
5 checks passed
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

Successfully merging this pull request may close these issues.

3 participants