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

[API Proposal]: Utf8JsonWriter.WriteNumberValue<TNumber> #103905

Open
dellamonica opened this issue Jun 24, 2024 · 4 comments
Open

[API Proposal]: Utf8JsonWriter.WriteNumberValue<TNumber> #103905

dellamonica opened this issue Jun 24, 2024 · 4 comments
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation area-System.Text.Json
Milestone

Comments

@dellamonica
Copy link

Background and motivation

Use the new generic number types in the JSON API.
Motivation: we can write higher level methods on top of the JsonWriter, for instance, to write an array of numeric values without having to repeat the same code block for each numeric type.

API Proposal

public void WriteNumberValue<TNumber>(TNumber number) where TNumber: INumber<TNumber>

API Usage

public void WriteArray(Utf8JsonWriter jw, ReadOnlySpan<TNumber> values) where TNumber: INumber<TNumber>
{     
        jw.WriteStartArray();
        foreach (TNumber val in values)
        {
            jw.WriteNumberValue(val);
        }
        jw.WriteEndArray();
}

...

WriteArray(jw, [3.14f, 2.71f, 0f]);
WriteArray(jw, [1, 2, 3]);

Alternative Designs

No response

Risks

No response

@dellamonica dellamonica added the api-suggestion Early API idea and discussion, it is NOT ready for implementation label Jun 24, 2024
@dotnet-policy-service dotnet-policy-service bot added the untriaged New issue has not been triaged by the area owner label Jun 24, 2024
Copy link
Contributor

Tagging subscribers to this area: @dotnet/area-system-text-json, @gregsdennis
See info in area-owners.md if you want to be subscribed.

@eiriktsarpalis
Copy link
Member

We'd probably also need a Utf8JsonReader.ReadNumber<T>() where TNumber: INumber<TNumber> as well. Sounds doable, INumber implements both IUtf8SpanFormattable and IUtf8SpanParsable.

cc @tannergooding

@eiriktsarpalis eiriktsarpalis removed the untriaged New issue has not been triaged by the area owner label Jun 24, 2024
@eiriktsarpalis eiriktsarpalis added this to the 10.0.0 milestone Jun 24, 2024
@Clockwork-Muse
Copy link
Contributor

.... Some recommendations around currency and JSON often say that such numbers should be serialized as string values, not as "raw" JSON numbers (which can silently end up as binary floating point in some circumstances). Given decimal implements INumber, that might have some adverse effects.

@elgonzo
Copy link

elgonzo commented Jun 24, 2024

such numbers should be serialized as string values, not as "raw" JSON numbers (which can silently end up as binary floating point in some circumstances)

Writing numbers as string values is not the concern of the existing Utf8JsonWriter.WriteNumber and WriteNumberValue methods, it would be inconsistent and confusing when WriteNumberValue<TNumber> happens to write a json string and not a json number (not to mention that a method name like "WriteNumberValue" would then be a rather misleading name for a method that doesn't write a number value). If you need to workaround poor json reader/deserialization routines or target environments lacking decimal number types other than binary floating points, a custom json converter that writes decimals as json strings is in my opinion a more appropriate choice to address this issue.

Given decimal implements INumber, that might have some adverse effects.

Considering that there are already WriteNumber/WriteNumberValue methods for decimals which write them as json numbers, I fail to see how a generic WriteNumberValue<TNumber> method writing decimals as a json number would introduce any adverse effects.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation area-System.Text.Json
Projects
None yet
Development

No branches or pull requests

4 participants