ohler55 / ojg

Optimized JSON for Go
MIT License
857 stars 49 forks source link

Remove nth element of an array using jsonPath #101

Closed denouche closed 1 year ago

denouche commented 1 year ago

Hello, I try to remove the nth element of an array.

I have this jsonPath:

$.services[0]

And my initial resource looks like:

{
  "services": [
    {
      "name": "service 1",
    },
    {
      "name": "service 2",
    }
  ]
}

I have my initial resource as a byte[] in b, and then I do this:

    compiledPath, _ := jp.ParseString("$.services[0]")

    var resourceAsMap interface{}
    _ = json.Unmarshal(b, &resourceAsMap)

    _ = compiledPath.Del(resourceAsMap)

(I removed the error handling for lisibility here)

I expected to have this after the Delete:

{
  "services": [
    {
      "name": "service 2",
    }
  ]
}

But instead of this I have:

{
  "services": [
    null,
    {
      "name": "service 2",
    }
  ]
}

When debugging I see that the null comes from here: https://github.com/ohler55/ojg/blob/7e29c4a1d4424608c77ed9f2dfdf2d3623572293/jp/set.go#L204

My question is, does the Del function should set the element to nil in the case of an array, or removing it? Thank you in advance for your answer!

ohler55 commented 1 year ago

The intent is to remove the element by setting it to null in a list so that the position of the remaining elements stay in their original position. Honestly I struggled with this one. In the end I thought it was better to preserve the location of the remaining elements over changing the list. That also avoided the issue of what to do if an element is removed from a top level list since the list could move or change without the original being modified in the expected way.

I can see a need for removing an element from a list so maybe a different function with a more narrow definition and some restrictions.

denouche commented 1 year ago

Thank you for your answer, it's clear.

Yes I think it could be interesting to be able to remove an element from a list, and I agree it's a better idea to create a new function for this (and avoid breaking the current behavior of the Del function).

ohler55 commented 1 year ago

Maybe call the functions "Remove" and have it return the original with elements removed and list shortened as needed.

There is another conversation going on in issue #99 as well but I'm not exactly sure what the desire is there. It might make sense to add more than one new function depending on outcome of that issue.

ohler55 commented 1 year ago

BTW, the development of the Remove() functions is in progress.

ohler55 commented 1 year ago

Please give the "jp-remove" branch a try.

denouche commented 1 year ago

Everything seems to be good. I tested the removal of an element in a array, and the removal of a string in a map. Thank you!

ohler55 commented 1 year ago

Released v1.16.0

denouche commented 1 year ago

wow! thank you so much for this release, for this project and for your time.