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

Json Decimal Separator #137

Open
muratyuceer opened this issue Nov 21, 2023 · 4 comments
Open

Json Decimal Separator #137

muratyuceer opened this issue Nov 21, 2023 · 4 comments

Comments

@muratyuceer
Copy link
Contributor

Hi,

How can I determine the decimal separator without overriding the global culture when deserializing JSON? As a matter of fact, I am asking to learn the most elegant approach to this question, thanks.

@Choc13
Copy link
Collaborator

Choc13 commented Nov 21, 2023

Hi @muratyuceer,

That's a good question. In this particular case the library defines a JsonConfigurationParser that is set as the default configuration parser. This is nothing more than a simple wrapper around the JsonStreamConfigurationProvider that Microsoft provides in their Microsoft.Extensions.Configuration.Json package, which in turn defers all of it's logic to an internal class called JsonConfigurationFileParser.

This is using System.Text.Json to parse a JsonDocument, but it's not deserialising it in the way you might be used to from deserialising JSON in a web api etc. Instead what it's doing is parsing it into the generic IConfiguration format that all configuration providers are designed to work with. This is basically just an IDictionary<string, string>. Additional packages such as Microsoft.Extensions.Options are then typically used to bind this IConfiguration in a "strongly typed" way to domain specific config types defined by your app.

What all of this means is that, in order to support different cultures for things like parsing numbers, we actually need to look inside the Microsoft.Extensions.Options package to see how we can customise the binding behaviour for certain values. It's not actually a JSON parsing problem in this instance.

I don't know off of the top of my head how to customise the parsing culture of Microsoft.Extensions.Options, but I believe from memory it works by using TryParse type methods to convert strings to other scalar values like int, decimal and bool. So that would be a place to look to understand how those methods parse different decimal values out of the box.

On a more general note, I personally don't love the way that the Microsoft.Extensions.Options package does configuration binding to strong types for this sort of reason. It's a bit of a black box and uses a lot of reflection and makes a lot of assumptions about how to interpret strings into different types and it also just fails silently and fills values with null when parsing fails which then leads to unsafe NullReferenceExceptions being thrown later on in the app, which sort of defeats the whole "type-safe" mantra it was trying to provide in the first place.

I actually wrote an alternative binder called Symbolica.Extensions.Configuration.FSharp which providers a safe and composable API for declaring how an IConfiguration should be mapped to the domain specific config types in your app. It should technically be usable from C#, but it makes use of the Option and Result type from F# and also is designed primarily to be used in an F# computation expression to make it read more declaratively. However, the point is that you might want to investigate using custom options binding code if the default binder doesn't support the formats you have for your decimal values.

@muratyuceer
Copy link
Contributor Author

Hi @Choc13

Thank you for the detailed explanation; I asked the question to the right person, and you've even published a package related to it :). To prevent unexpected errors, I use this method along with the following https://learn.microsoft.com/en-us/aspnet/core/fundamentals/configuration/options?view=aspnetcore-8.0#validateonstart it helps to proactively address some potential runtime errors (For instance, I make every type nullable and add the required attribute).

I'm considering writing a code similar to this

char decimalSeparator = Convert.ToChar(CultureInfo.CurrentCulture.NumberFormat.NumberDecimalSeparator);
string value = str.Replace(',', decimalSeparator).Replace('.', decimalSeparator);

to filter out values with a type of decimal when binding to the model. I will add this code; I'm just researching the exact right place to include it. However, I'm also curious about your opinion.

@Choc13
Copy link
Collaborator

Choc13 commented Nov 22, 2023

Yes ValidateOnStart is a good thing to use if you're using the options pattern.

It depends on what you're exactly trying to do here. Are you trying to parse numbers that use ',' as the decimal separator? Or are you trying to convert all numbers to use the correct decimal separator for the current culture? And do you want to eventually parse these values as a decimal or do you want to keep them as strings but with the correct separators for the current culture in them? Does the config source not have a consistent culture to it?

@muratyuceer
Copy link
Contributor Author

Hi @Choc13

Actually, my goal is as follows: In settings files like appsettings or Consul, someone might have defined a value like TransactionLimit: 10,50 or TransactionLimit: 10.50, and if the class property to which this value will be bound is of type decimal, I want to replace these values first with the CurrentCulture decimal separator to avoid errors.

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

No branches or pull requests

2 participants