Mutation support

[zoul]

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"] = baz syntax 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 using let and 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 like updating(key:to:)), allowing the callers to pick whatever is best for them.

[zoul]

In-Place Mutation

This 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 Types

One catch is what should be done when the JSON value is not compatible with the change, ie. when someone tries to change the foo key on a JSON float or write to the first index of a JSON dictionary:

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 self to the expected JSON value type:

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 Access

Since it’s syntactically valid to do JSON([])[10] = 1, what does it mean? Should it be a hard out-of-bounds error like with ordinary Swift array accesss, a no-op, or should we grow the array automatically? I’m against a hard error, since the value often comes from an external source and it would be harsh to crash an app just because an external API source forgot to include an expected value in a response.

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 Dictionaries

The syntax is obvious again:

json["foo"] = nil
json.foo = nil

It’s not obvious what whould be done when json is not a dictionary. One option is to do nothing, one option is to convert the value to a dictionary first:

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 Arrays

The json[0] = nil syntax really means json[0] = JSON.null. Writing to an out-of-bound index should be ignored, but the value would be converted to an array if not an array already, just as in a dictionary.

---

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?

[cjmconie]

Thanks for putting this together, and apologies for the delay.

And if people want to stick to immutable values, they can always just declare their JSON values using let and that’s it.

Yes, that makes sense. The choice of immutability is shifted to the consumer – which I think is fine.

Incompatible JSON Types

And a reasonable default behaviour could be to simply change self to the expected JSON value type

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, JSON, does not change but the internal 'type' (i.e. enum case) changes.

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 .string

the case has changed from .number to .string – which is reasonable.

However, here:

var foo: JSON = 1 // foo is .number
foo.a = "b" // foo is now .object {"a": "b"}

the type is being "upgraded" from .number to .object as a result of a subscript assignment.

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

Since it’s syntactically valid to do JSON([])[10] = 1, what does it mean? Should it be a hard out-of-bounds error like with ordinary Swift array accesss, a no-op, or should we grow the array automatically? I’m against a hard error, since the value often comes from an external source and it would be harsh to crash an app just because an external API source forgot to include an expected value in a response.

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.

With help from my Android colleague, @lordcheddar , we inspected what similar libraries do.

JSON-java

• Adds more storage (grows the array with json nulls)

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)}")
    }
  }

JSON
2019-06-24 13:31:31.780 11671-11671/com.ydangleapps.jsontest E/json: index 0 - null
2019-06-24 13:31:31.780 11671-11671/com.ydangleapps.jsontest E/json: index 1 - null
2019-06-24 13:31:31.780 11671-11671/com.ydangleapps.jsontest E/json: index 2 - null
2019-06-24 13:31:31.780 11671-11671/com.ydangleapps.jsontest E/json: index 3 - null
2019-06-24 13:31:31.780 11671-11671/com.ydangleapps.jsontest E/json: index 4 - null
2019-06-24 13:31:31.780 11671-11671/com.ydangleapps.jsontest E/json: index 5 - test

GSON

• Runtime crash

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}")
    }
  }

GSON - java.lang.IndexOutOfBoundsException: Index: 5, Size: 0

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 Dictionaries

In this context, and from what I can tell (again no expert), it comes down to how the language's nil/null concept translates to JSON's null.

Setting an object's value to language's nil/null will either:

1. remove the key pair, or
2. replace the value with JSON's null.

JSON-java

• Assigning language's null to a dictionary value will remove the key pair.
• In order to assign null to a value, the JSON type's null value must be used.

  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)}")
    }
  }

2019-06-24 13:38:12.843 13524-13524/com.ydangleapps.jsontest E/json: {"array":["test"]}
2019-06-24 13:38:12.843 13524-13524/com.ydangleapps.jsontest E/json: {}

GSON

• Assigning language's null to a dictionary value will replace the value with null.
• Removing the key-pair entirely must be done using remove().

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())
  }

2019-06-24 13:38:12.860 13524-13524/com.ydangleapps.jsontest E/gson: {"array":["test"]}
2019-06-24 13:38:12.860 13524-13524/com.ydangleapps.jsontest E/gson: {"array":null}
2019-06-24 13:38:12.861 13524-13524/com.ydangleapps.jsontest E/gson: {}

In the proposed method, we are using the Swift language's nil to remove key-pairs:

The syntax is obvious again:

json["foo"] = nil json.foo = nil

I see one inconsistency here, currently initialisation using Swift's nil produces JSON.null:

let foo = JSON(nil) // .null

and the proposal for 'Removing Items From Arrays' treats a nil assignment as JSON.null assignment.

Given this, would it make more sense to add an explicit remove() function (similar to GSON) which removes key-pairs? So given var foo: JSON = ["bar" : true] these two assignments would be equivalent:

foo.bar = .null
foo.bar = nil

Do these comments help narrow the solution space?