Parse ByteSize with IFormatProvider?

[dahlbyk]

While poking around for #1060 I noticed CultureInfo.CurrentCulture in ByteSize.TryParse():

Humanizer/src/Humanizer/Bytes/ByteSize.cs

Line 474 in 1285835

var decSep = Convert.ToChar(CultureInfo.CurrentCulture.NumberFormat.NumberDecimalSeparator);

Would you be interested TryParse/Parse overloads that accept IFormatProvider? Asking first since it's a public API change.

It would check if the IFormatProvider is either CultureInfo or a NumberFormatInfo.

[clairernovotny]

Can we do it with a new overload? I'm okay with adding overloads; been trying to avoid breaking changes for the most part.

[dahlbyk]

Yeah, should have clarified I meant an additive public API change.

While we're here, there's also a reference to CurrentCulture here:

Humanizer/src/Humanizer/Transformer/ToLowerCase.cs

Lines 7 to 10 in 12cc400

	public string Transform(string input)
	{
	return CultureInfo.CurrentCulture.TextInfo.ToLower(input);
	}

But not here:

Humanizer/src/Humanizer/Transformer/ToUpperCase.cs

Lines 5 to 8 in 12cc400

	public string Transform(string input)
	{
	return input.ToUpper();
	}

Seems like they should be consistent? Is there any value in allowing a culture-specific transform?

[clairernovotny]

TBH, it's been so long since I've looked at that code. It does seem odd and I agree that CurrentCulture should be used where appropriate.

I'm not sure if having a culture aware one makes a difference, but I'm open to additional overloads that take the culture.

[dahlbyk]

TBH, it's been so long since I've looked at that code. It does seem odd and I agree that CurrentCulture should be used where appropriate.

ToUpper() uses CurrentCulture, so the behavior is close to the same. The only difference is a NullReferenceException vs ArgumentNullException given null.

At minimum we can fix the NullRef here and in ToSentenceCase. I would argue that since null handling isn't well-defined, it's just as useful for most transforms to short-circuit and return null instead of throwing.

I'm not sure if having a culture aware one makes a difference, but I'm open to additional overloads that take the culture.

Can't overload the To properties that return the given transformers, but I think it could work to do something like this:

// Better name ideas?
public interface ICulturedStringTransformer : IStringTransformer
{
    string Transform(string input, CultureInfo culture);
}

With a new overload:

        public static string Transform(this string input, CultureInfo culture, params ICulturedStringTransformer[] transformers)
        {
            return transformers.Aggregate(input, (current, stringTransformer) => stringTransformer.Transform(current, culture));
        }