DefaultUrlParameterFormatter uses the culture specific ToString method to convert parameters. When using Refit with certain cultures (e.g. in a mobile app), even a double will be serialized inconsistently (e.g. with a comma instead of a period). In general, it's bad form to use ToString for "serialization". Culture specific ToString is particularly bad.
This has bit me enough times, that I now use a custom IUrlParameterFormatter every single time.
Does it ever make sense for this to be culture dependent?
Would it make sense to change the default (possibly breaking Refit for those who depend on it)?
Otherwise, maybe adding something like my InvariantUrlParameterFormatter to Refit would be helpful.
I can setup a PR for any of the above, but would like some feedback before proceeding.
Yes, that's how IUrlParameterFormatter got added to Refit. And it's great that it exists, because that's what allows me to fix this on my own. Here, I'm wondering whether the default should be to call the culture specific ToString, and/or whether we should provide a utility class to help with this.
I mean, it may very well be too late to change the default, but what REST API depends on being invoked with the culture specific formatting of float/double/decimal?
The default _right now_ is it use a comma (not a period) as the decimal separator in cultures such as spanish, portuguese, russian, german, french, italian, romanian, dutch, just to name a few. That's millions of people/devices/etc. Does this _ever_ make sense for the query parameters of an API?
I regularly use Refit on mobile devices to invoke REST APIs that include location searches (latitude, longitude) and this bites me every single time, when it lands on a foreign device configured with a different culture. How many of you are/were aware of this?
Agreed. Has bitten me a couple of times over. In my opinion it would never make sense.
Then again, formatting things like DateTime would be better in iso8601 instead of the culture invariant approach. It's good we can implement it ourselves though.
@ncruces thumbs up for the converter. And yes the conversion should be CultureInvariant. +1.
@tehZeno I agree regarding ISO 8601, but at least the behavior is consistent, and you realize there's a problem before you ship.
Making this depend on the device's culture increases the probability that you're testing in with a en-US device and never notice a problem before it lands in a differently configured device.
I'm okay with making this a breaking change for v4.0
This is fixed in 4.1 and invariant is now the default.
Most helpful comment
This is fixed in 4.1 and invariant is now the default.