search

fsprojects / Fleece

Json mapper for F#
http://fsprojects.github.io/Fleece
Apache License 2.0
199 stars 31 forks source link

Use a different type alias for JsonObject #81

Closed gusty closed 4 years ago

gusty commented 4 years ago

Until now, Fleece for FSharp.Data defined JsonObject as a (string * JsonValue) [].

This implies that depending on how do we interpret it, could be serialized either as JObject or JArray which might result in problems.

The idea is to use a type alias to a type that already serialize to JObject.

I propose to use Map<string,JsonValue>.

We could consider IReadOnlyDictionary<string,JsonValue>, which is used internally and we don't have an overload for it, but note that being an interface, we need to assess first that no undesired subsumption would happen in the overload resolution.

Note that this might break things in projects using FSharp.Data, but for System.Text.Json there won't be problems as long as this change is included in its first release.

Finally, I don't know why the type alias to (string * JsonValue) [] was choosen, not sure if @eulerfx can remember the reason and give some feedback.

wallymathieu commented 4 years ago

I think it would be a good change since the (string * JsonValue) [] is a more raw representation of the json format. Any deserialization into values will use just one of the values if there they have the same key.

gusty commented 4 years ago

The concern about multiple keys is also there. At first sight I thought that had been one of the reasons for (string * JsonValue) [], since Json in theory allows multiple keys, but even in that case as you said, it would at some point use a dictionary and would use just one of the keys.

If we really want to support multiple keys we should change drastically the design and use a MultiMap, in all 4 implementations, but still I'm not sure all 4 libs (as standalone) support multiple keys.

gusty commented 4 years ago

Closed by #82