Add overload for specifying the culture for Ordinalize() extension method

[tiesmaster]

Most extension methods have support for overriding the culture, however Ordinalize() doesn't have that. For example, ToOrdinalWords() gives you the opportunity to specify it, like this:

    10.ToOrdinalWords(new CultureInfo("nl-NL")) => "tiende"

It would be nice if you have that overload for Ordinalize() as well.

[aloisdg]

Hello,

Indeed, it could be nice. Would you submit a PR for this?

[tiesmaster]

Cool, I just had a look. Though, I was wondering about how to add it. Although, I only need a new extension method to specify the CultureInfo for the Ordinalize(int) method, I've noticed that there are a number of overloads (4 overloads, to be exact) for this extension method for accepting a string, and Gender.

Shall I only add a new one: Ordinalize(int, CultureInfo), or add a new overload for overload that is already there, duplicating the amount of overloads with that.

I can also add CultureInfo as optional parameter, and make it null by default, like in NumberToWordsExtension. However, that breaks consumers, since that's a breaking API change, and results in MissingMethodExceptions.

[tiesmaster]

I've got some preliminary work. How does that look?

[aloisdg]

You opened a nice question and I think we shouldnt break here. I think we need more overview (cc @onovotny @hazzik ). I might be not good enough. Your solution looks good to me. Can you open the PR (so we can track it, etc.)?

[tiesmaster]

So, if I understand you correctly, you might want to go the route of adding CultureInfo to all overloads (either with optional arguments, or by making sure backwards compat maintains), but you want me to put the commit in a PR, so we can start off with discussing it there. Sure, I can do that.

BTW Roslyn follows this mechanism for adding new optional arguments to methods. Maybe, that might work here, as well.

[AKTheKnight]

@onovotny looks like the PR for this #595 was closed and never merged.

What are your thoughts on me submitting a new PR for this? And do you remember if there was a conversation had about adding CultureInfo to all overloads/methods where appropriate?

[clairernovotny]

Happy to take a PR adding a CultureInfo overload to API's that don't have one.

[tiesmaster]

I think I closed the PR back then because of inactivity on it. I've reopened it, and rebased changes on latest master. This is only the change I did for the overload I needed back then, but I intent to add the other ones, when I see the CI pass on my original changes.

[tiesmaster]

@onovotny Added the missing overloads. Please have a look at the re-opened PR #595