Skip to content

[API Proposal]: Add JsonElement.ValueEquals(JsonElement) #99994

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Open
bill-poole opened this issue Mar 20, 2024 · 9 comments
Open

[API Proposal]: Add JsonElement.ValueEquals(JsonElement) #99994

bill-poole opened this issue Mar 20, 2024 · 9 comments
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation area-System.Text.Json
Milestone

Comments

@bill-poole
Copy link

Background and motivation

The System.Text.Json.JsonElement struct currently provides ValueEquals overloads for comparing the value of a JsonElement with a string (string, ReadOnlySpan<char> or ReadOnlySpan<byte>) representation of another JsonElement instance. However, if we want to compare two JsonElement instances, then this requires the following, which requires heap allocations and UTF-8 decoding.

jsonElement1.ValueEquals(jsonElement2.GetString());

It would be presumably much more efficient to be able to do:

jsonElement1.ValueEquals(jsonElement2);

API Proposal

namespace System.Text.Json;

public readonly struct JsonElement
{
    public bool ValueEquals(JsonElement other);
}

API Usage

// Compare jsonElement1 with jsonElement2
Console.WriteLine($"Values are equal: {jsonElement1.ValueEquals(jsonElement2)}");

Alternative Designs

No response

Risks

No response

@bill-poole bill-poole added the api-suggestion Early API idea and discussion, it is NOT ready for implementation label Mar 20, 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.

@dotnet-policy-service dotnet-policy-service bot added the untriaged New issue has not been triaged by the area owner label Mar 20, 2024
@gregsdennis
Copy link
Contributor

I have this as an extension in Json.More.Net.

var isEqual = elt1.IsEquivalentTo(elt2);

The code is here if you want to inspect.

@bill-poole
Copy link
Author

Thanks @gregsdennis!

Your implementation still allocates though if the two JsonElement instances are JsonValueKind.String, JsonValueKind.Array or JsonValueKind.Object. I'm hoping a JsonElement.ValueEquals(JsonElement) overload would be allocation-free.

Also, comparing two JsonElement instances of type JsonValueKind.Number by parsing them both into Decimal instances is lossy if the two numbers have more decimal places than supported by the System.Decimal type.

@gregsdennis
Copy link
Contributor

Also, comparing two JsonElement instances of type JsonValueKind.Number by parsing them both into Decimal instances is lossy if the two numbers have more decimal places than supported by the System.Decimal type.

This is true regardless of what .Net numeric type you use. It's an inherent consequence of a limited practical numeric space vs JSON's unlimited theoretical numeric space.

I chose decimal because I think my clients would prefer the extra precision over the extra range of double.

@bill-poole
Copy link
Author

This is true regardless of what .Net numeric type you use.

Yes, but I would imagine a JsonElement.ValueEquals(JsonElement) overload would compare the numbers as strings, rather than converting them into a binary floating point type with limited precision/range.

@gregsdennis
Copy link
Contributor

gregsdennis commented Mar 20, 2024

Yeah, that doesn't work, either: #97490.

4.0 and 4 are both valid JSON representations of the same value, but a string comparison doesn't work.

@bill-poole
Copy link
Author

But isn't that the current behavior of JsonElement.ValueEquals(String), JsonElement.ValueEquals(ReadOnlySpan<char>) and JsonElement.ValueEquals(ReadOnlySpan<byte>)? I looked at their implementations and they don't deserialize then compare, they just compare the strings.

I would therefore propose that JsonElement.ValueEquals(JsonElement) would behave the exact same way. i.e., jsonElement1.ValueEquals(jsonElement2) would do exactly the same thing as jsonElement1.ValueEquals(jsonElement2.GetString()), but without the unnecessary allocation/decode.

@eiriktsarpalis
Copy link
Member

Duplicate of #33388. Note that this has already been implemented in .NET 8 for JsonNode via the JsonNode.DeepEquals method.

@dotnet-policy-service dotnet-policy-service bot removed the untriaged New issue has not been triaged by the area owner label Mar 20, 2024
@gregsdennis
Copy link
Contributor

@eiriktsarpalis I think the ability to do this without allocation on JsonElement would be valuable. I don't think this is the same as that issue you linked.

@dotnet-policy-service dotnet-policy-service bot added the untriaged New issue has not been triaged by the area owner label Mar 20, 2024
@eiriktsarpalis eiriktsarpalis added this to the Future milestone Mar 20, 2024
@jeffhandley jeffhandley removed the untriaged New issue has not been triaged by the area owner label Mar 21, 2024
# for free to join this conversation on GitHub. Already have an account? # 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