kotlinx.serialization icon indicating copy to clipboard operation
kotlinx.serialization copied to clipboard

Update JSON builders to provide more mutable/iterable operations

Open aSemy opened this issue 1 year ago • 3 comments

What is your use-case and why do you need this feature?

The JSON array and object builders are very useful, but their operations are restricted to only add elements. This makes it more difficult to perform additional mutations (like removing elements), or collection-based operations like iterating over the values.

Implementing all of the possible List-specific and Map-specific operations can be easily achieved via delegation.

Missing operations require in unclear workarounds

For example, there's no a clear way of how to remove elements at the moment. Without a default method method, it's easy to

  1. come up with complicated workarounds that aren't easy to read, or scale

    val obj = buildJsonObject {
      put("a", 1)
      put("b", 2)
      put("c", 3)
    }
    println(obj)
    
    println(
      // if I'm in a rush I might quickly hack this, which is confusing
      // and incurs technical/documentation debt
      JsonObject(obj.toMap().run { minus("b") })
    )
    
    println(
      // if I had more time, this is a little more clear
      JsonObject(obj.toMap() - "b")
      // but it requires some deeper understanding how Kotlin/KxS works,
      // more than should be necessary for a basic operation
    )
    
  2. accidentally write code that looks correct, but introduces bugs

    println(
      // On first glance this looks like it might work
      obj.toMutableMap().minus("b")
      // but it returns a MutableMap, not a JsonObject
    )
    
  3. or (for Kotlin / KxS beginners) get confused and give up!

Mutable/immutable JSON element mismatch

There is also a mis-match between the JSON builders and the read-only classes they build.

  • JsonObject implements Map<String, JsonElement> via delegation https://github.com/Kotlin/kotlinx.serialization/blob/7a35a2d2d7f817930b43dd5e07f318ba3b99d674/formats/json/commonMain/src/kotlinx/serialization/json/JsonElement.kt#L191-L194

  • JsonArray implements List<JsonElement> via delegation https://github.com/Kotlin/kotlinx.serialization/blob/7a35a2d2d7f817930b43dd5e07f318ba3b99d674/formats/json/commonMain/src/kotlinx/serialization/json/JsonElement.kt#L220

It makes sense to update the JSON builders so they work similarly, which helps with understanding and reduces the difference between the two.

Sharing methods

It would also help understanding if JsonObject/JsonArray shares as much functionality as possible with MutableMap/MutableList, because learning one helps with learning to use the other.

Describe the solution you'd like

  • introduce MutableJsonArray and MutableJsonObject classes
  • MutableJsonArray extends JsonArray and implements MutableList<JsonElement> using delegation
  • MutableJsonObject extends JsonObject and implements MutableMap<String, JsonElement> using delegation
  • Remove JsonArrayBuilder, and replace it with the new MutableJsonArray
  • Remove JsonObjectBuilder, and replace it with the new MutableJsonObject

Example:

public class JsonArrayBuilder @PublishedApi internal constructor(
    private val content: MutableList<JsonElement> = mutableListOf()
): MutableList<JsonElement> by content

I also see two further improvements:

Mutable variants

Currently the JSON object/array builders are internal and are only used for the builder operations, but why is this? Since they would become specialised implementations of a mutable collection, why not make them publicly accessible? I propose the following:

  • Rename JsonArrayBuilder to MutableJsonArray,
  • and JsonObjectBuilder to MutableJsonObject,
  • and update the constructors to be public.

This would allow users to create and define mutable JSON objects and arrays, using a class that would also be shared with the buildJsonObject {} and buildJsonArray {} helpers.

Update extension functions?

If JsonArrayBuilder implements MutableList<JsonElement>, then all of the JsonArrayBuilder extension functions could be adapted to extend MutableList<JsonElement> instead of JsonArrayBuilder.

public fun MutableList<JsonElement>.add(value: Number?): Boolean = add(JsonPrimitive(value))

This would improve the usability of KxS in more situations, making the KxS more convenient to use.

See also

  • I wonder if we should just add addAll or simply make JsonArrayBuilder implement MutableList (by delegation, like with JsonArray or by implementing AbstractMutableList)

    Originally posted by @sandwwraith in https://github.com/Kotlin/kotlinx.serialization/issues/2127#issuecomment-1387506383

  • It would be nice to provide builders that comply with such expectations and where remove* functions are added deliberately (e.g. not buildList {} where it just comes with the MutableList contract), and the overall prior art overview.

    For example, Guava, which is arguably one of the most used libraries, doesn't provide remove* with their builders (https://github.com/google/guava/issues/3596) , Scala's builder doesn't seem to have one and so on

    Originally posted by @qwwdfsad in https://github.com/Kotlin/kotlinx.serialization/issues/2191#issuecomment-1431990450

aSemy avatar May 18 '23 15:05 aSemy

Upvote.

I found mutable JSON builders would simplify a lot of code in my project and adding them shouldn't require much work. For now I have to store everything inside MutableList<JsonElement> or MutableMap<String, JsonElement> to make it mutable.

Hope the developers can release the newer package ASAP.

MqCreaple avatar May 31 '23 04:05 MqCreaple

After further thought I'm much more keen on the idea of introducing MutableJsonArray and MutableJsonObject classes. This would provide more functionality, and more closely matches how MutableList/List and MutableMap/Map work in the stdlib.

Should I make a new issue for this request @sandwwraith, or can I re-write this one?

aSemy avatar May 31 '23 07:05 aSemy

Upvote too, it's very inconvenient to modify json without model class.

zjns avatar May 19 '24 07:05 zjns