add function for removing entry in `JsonObjectBuilder`

[TheFruxz]

trafficstars

Adds JsonObjectBuilder.remove(key: String) to remove an entry from a JsonObjectBuilder instance.

My first PR to this repo, so I hope I did everything right. :D

[qwwdfsad]

Could you please provide a rationale and the reasoning for the proposed API? What are practical use-cases?

[TheFruxz]

Firstly, I would expect from a builder class, that it can remove stuff, if there is no technological barrier against it. But it is also a good thing to have, if you have an external API adding a bunch of stuff to it (or iterative based operations) because then you can remove specific stuff after it.

For example: I have a reader, reading a file or user input, which reads line per line. Then a line after the first line is telling something that the first line is obsolete or not required. Then I would have to remove something, but with this builder, this is currently not possible.

Another example here would be, that you want to have an identifier at the beginning of the JSON, but it is not required in all cases, if the content added later would lead to a conflict-free store.

Every of these cases would lead to much cleaner code, if the builder has the ability to simply remove entries after they got added.

I would appreciate feedback if I am wrong about the functionality.

[sandwwraith]

@qwwdfsad Given this request and #2156 , I think it is worth considering that builders should implement MutableMap/List just like regular JsonElements implement Map/List

[qwwdfsad]

Mutable* is another side of the story -- we could do that, and it would probably be a reasonable thing to do (after a rehearsal of our design notes regarding stdlib).

The issue with the proposed PR is that it doesn't solve any real-world problems, only theoretical ones.

Firstly, I would expect from a builder class, that it can remove stuff, if there is no technological barrier against it.

Unfortunately, this is different from how we are inclined to think about API addition. We need a compelling reason to add stuff, not the other way around, and the score of the reasoning starts at negative values.

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

[aSemy]

Could you please provide a rationale and the reasoning for the proposed API? What are practical use-cases?

One reason is that there's not a clear way of how to achieve this at the moment. Without a clear helper 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!

4. It would also help understanding if JsonObject shared as much functionality as possible with MutableMap, because learning one helps with learning to use the other.

I think it is worth considering that builders should implement MutableMap/List just like regular JsonElements implement Map/List

I agree, JsonObject should implement MutableMap and JsonArray should implement MutableList.

[wakaztahir]

I think the original objects should be mutable , JsonObject , I have a heirarchy of objects , I navigate deeply into nested object using a Json Pointer , Json Pointer , a class I created which holds segments like "obj1/obj2/key" in List<String> and then navigates to it , If I want to modify that pointer I will not only create a copy of this object , I will have to go up in the heirarchy and create copies of objects and change it , to insert a single key at that pointer

I think just like there's a List<String> and MutableList<String> there should be JsonObject and MutableJsonObject

Currently I use

internal fun Map<String, JsonElement>.withValue(key: String, value: JsonElement): JsonObject {
    return JsonObject(toMutableMap().apply { put(key, value) })
}

internal fun Map<String, JsonElement>.withValue(pointer: JsonPointer, value: JsonElement): JsonObject {
    if (pointer.segments.size == 1) return withValue(pointer.segments.first(), value)
    val root = toMutableMap()
    val parent = JsonPointer(pointer.segments.dropLast(1)).navigateToMutable(root)
    parent[pointer.segments.last()] = value
    return JsonObject(root)
}

 fun navigateToMutable(elements: MutableMap<String, JsonElement>): MutableMap<String, JsonElement> {
      var current = elements
      for (segment in segments) {
          current = (current[segment] as? JsonObject)?.toMutableMap() ?: mutableMapOf<String, JsonElement>().also {
              current[segment] = JsonObject(it)
          }
      }
      return current
  }

[aSemy]

I've created a new issue to take this proposal further: https://github.com/Kotlin/kotlinx.serialization/issues/2308