Lack of consistency + lack of awareness re. cultural processing of strings #311
Description
Looking at the code base as a whole, there seems to be a strong lack of consistency when it comes to dealing with cultural variation wrt the processing of strings. E.g. some people use s.ToUpper(), while others, perhaps knowing better, use s.ToUpperInvariant().
Some people use #if FX_NO_CULTURE_INFO_ARGS to determine whether to specify culture info and/or StringComparison args to standard .NET methods, while most others don't.
Also, there seems to be a complete lack of awareness of the part of many contributors about the whole issue of a) cultural vs non-cultural conversion of strings, and b) current culture vs invariant culture vs ordinal (with/without ignore case) comparison of strings.
As it stands, FSharpx is a library I would be fearful to use. Looking at the code, I know that I cannot predict the outcome of many operations when run on other-culture (e.g. non-English) machines.
It has been drummed into me over the last 10+ years the critical importance of always stating your cultural / ordinal intent when it comes to string conversion and comparison.
I would suggest a cleanup throughout the code, along these lines...
- For string conversions, always state your cultural (or non-cultural) intent. E.g. if you wish to convert a string to upper case, either use s.ToUpperInvariant() or the overload of ToUpper that takes a CultureInfo arg. Never use the s.ToUpper() overload.
- Avoid methods like s.Contains() which only work in a culture-sensitive and case-sensitive manner (use s.IndexOf instead).
- For string comparisons (e.g. s.StartsWith, s.EndsWith, s.Equals), always use an overload of the method that takes a StringComparison enum value, and carefully consider in each case whether your comparison should use the current culture, the invariant culture (regionless English) or - by far the most common choice in data processing - ordinal. Also give consideration to case-sensitivity.
- The use of #if FX_NO_CULTURE_INFO_ARGS should be deprecated in favour of always specifying culture info args (and the like).
- When a function is public (e.g. "startswith") and needs an extra arg (e.g. "comparisonType") to bring it into line, this will be a breaking change. But a worthwhile one, imo. All .NET developers, including F# developers, need to be aware of this stuff and to give it serious consideration at all times.