Mutation support #26
Comments
In-Place MutationThis is how in-place mutation could look from the caller’s perspective: // Note `var` instead of `let`
var json: JSON = …
// Writable key subscript
json["foo"] = bar
// Writable index subscript
json[0] = baz
// Dynamic member subscript
json.foo = bar
// In-place merge
json.merge(["foo": "bar"])Incompatible JSON TypesOne catch is what should be done when the JSON value is not compatible with the change, ie. when someone tries to change the var foo: JSON = 1
foo.a = "b" // now `foo` is what?We had some discussion around that in #25 and it seems that there are three options – design the types to make these operations impossible, signal an error, or use a reasonable default behaviour (possibly including doing nothing). Changing the types seems a little heavy-handed and returning an error would be hard to do (how do we return an error when using an invalid subscript write?), so it seems that the last option is best. And a reasonable default behaviour could be to simply change JSON(1).a = "b" // {"a": "b"}
JSON(1)[0] = "x" // ["x"]I’m not really happy about it, since it makes it too easy for people to shoot themselves in the foot, but I can’t think of a better solution. And at least it’s consistent with how merging works at the moment. Out-Of-Bound Array AccessSince it’s syntactically valid to do Growing the array automatically would be a bit closer to how arrays behave in JavaScript. But I think that’s a false analogy anyway, so I would tend to pick the remaining option and not do anything – except for converting the value to array if needed, see remark below about removing keys from a dictionary. Removing Keys From DictionariesThe syntax is obvious again: json["foo"] = nil
json.foo = nilIt’s not obvious what whould be done when JSON(1).foo = nil // 1
JSON(2).foo = nil // {}The first choice is consistent with how key reading from non-dictionary values works, the second choice is similar to how key writing works. I’m in favour of the second one. Removing Items From ArraysThe I have left out the immutable update API. I think we can get to that later, as it will only be a thin wrapper above the mutable one (create a mutable copy, run a mutating change, return). @ezamagni, @cjmconie, you both wanted mutation support, how does this look to you? Is anything missing, inconsistent, surprising, wrong? |
|
Thanks for putting this together, and apologies for the delay.
Yes, that makes sense. The choice of immutability is shifted to the consumer – which I think is fine. Incompatible JSON Types
This is a tough one. It would follow the dynamically typed origins of JSON, Javascript. That said, it does make for a tricky API. To the caller, the variable type, I can see how this would make complete sense to the caller: var foo: JSON = 1 // foo is .number
foo = JSON("hello") // foo is now .stringthe case has changed from However, here: var foo: JSON = 1 // foo is .number
foo.a = "b" // foo is now .object {"a": "b"}the type is being "upgraded" from My view that we should not allow the "upgrading" of type as a result of a subscript assignment if the underlying case does not logically support it. Out-Of-Bound Array Access
With help from my Android colleague, @LordCheddar , we inspected what similar libraries do.
fun jsonArrayTest() {
val temp = JSONObject()
temp.put("array", JSONArray().put(5, "test"))
val array = temp.getJSONArray("array")
(0 until array.length()).forEach {
Log.e("json", "index $it - ${array.opt(it)}")
}
}
fun gsonArrayTest() {
val temp = JsonObject()
val jarray = JsonArray(6)
jarray.set(5, JsonPrimitive("test"))
temp.add("array", jarray)
val array = temp.get("array").asJsonArray
(0 until array.size()).forEach {
Log.e("gson", "index $it - ${array.get(it).asString}")
}
}I am not sure what the best approach is here, however as you mentioned, I also think the runtime exception is not a good choice. Removing Keys From DictionariesIn this context, and from what I can tell (again no expert), it comes down to how the language's Setting an object's value to language's
fun jsonArrayTest() {
val temp = JSONObject()
temp.put("array", JSONArray().put(5, "test"))
val array = temp.getJSONArray("array")
(0 until array.length()).forEach {
Log.e("json", "index $it - ${array.opt(it)}")
}
}
fun gsonTypeNullTest() {
val temp = JsonObject()
val jarray = JsonArray(6)
jarray.add(JsonPrimitive("test"))
temp.add("array", jarray)
Log.e("gson", temp.toString())
temp.add("array", null)
Log.e("gson", temp.toString())
temp.remove("array")
Log.e("gson", temp.toString())
}In the proposed method, we are using the Swift language's
I see one inconsistency here, currently initialisation using Swift's let foo = JSON(nil) // .nulland the proposal for 'Removing Items From Arrays' treats a Given this, would it make more sense to add an explicit foo.bar = .null
foo.bar = nilDo these comments help narrow the solution space? |
The demand for mutation support is obvious (#6, #25). I was trying to stick to immutability, but if people need to patch a JSON value, the
foo["bar"] = bazsyntax is very hard to beat – it’s intuitive, discoverable and terse. And if people want to stick to immutable values, they can always just declare their JSON values usingletand that’s it. So I decided we should give in and offer both a mutable API (writable subscripts,merge(with:)) and an immutable update API (merging(with:), something likeupdating(key:to:)), allowing the callers to pick whatever is best for them.The text was updated successfully, but these errors were encountered: