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

Additional Salutations #42

Open
jamessampford opened this issue Apr 23, 2020 · 6 comments
Open

Additional Salutations #42

jamessampford opened this issue Apr 23, 2020 · 6 comments

Comments

@jamessampford
Copy link

There are quite a few extra salutations that could be added (such as Lord, Lady, Dame)

Long list:
https://en.wikipedia.org/wiki/English_honorifics

@joeynovak
Copy link
Contributor

Definitely, and it doesn't appear that the code supports an easy way to add more salutations without creating a subclass.

@wyrfel
Copy link
Contributor

wyrfel commented May 6, 2020

@jamessampford Would you like to create a pull request?

@wyrfel
Copy link
Contributor

wyrfel commented May 6, 2020

@joeynovak How would you imagine the ability to add more/different salutations implemented?

@joeynovak
Copy link
Contributor

Sorry for the delay in getting back to you. A simple way would be to use a static array instead of a constant for the list, and just add a static method to add to the list. (I know a lot of developers hate anything static, but I find that unless you are running in an environment where multiple requests share the static scope (.Net, Java) it works great and doesn't produce bugs).

@HaroldPutman
Copy link
Contributor

HaroldPutman commented May 15, 2020

It is the same for suffixes. They are very domain specific - a healthcare app might need to have RN , LPN, LSW, etc...
Would it be too JavaScript-y to pass options into the constructor?

$parser = new TheIconic\NameParser\Parser([ 
    'suffixes' => ['lpn' => 'LPN', 'lsw' => 'LSW', 'rn' => 'RN'], 
    'salutations' => [ 'honourable' => 'Honourable']
]);

Oh never mind. Looking through the code and rtfm I see that's exactly how setting languages works.
Just make a HealthcareNames "Language" and add the Suffixes you need.

You might consider change the name from Language to Nomenclature. Just because we tend to thing of using only one Language at a time, so if I am already using English "Language" it seems odd to create a new "Language" to handle Healthcare names.

@wyrfel
Copy link
Contributor

wyrfel commented May 21, 2020

Thanks for the feedback @jamessampford @joeynovak and @HaroldPutman .
I believe the subclassing way of adding more definitions is a better mechanism than having setters routed through the parser or being statically available. After all, you would have to put code somewhere to call these setters and if you have that code somewhere, why not put it into a custom language class and pass that into the parser alongside the english language class (you can pass several language classes)?
This also allows you to write quite generic classes, e.g. one that has of those setters and getters. Or one that fetches definitions from a data store. Or from an API. Or from a configuration file.
I agree with the sentiment that 'Language' is not the perfect name for those classes and will adjust that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

4 participants