objconv icon indicating copy to clipboard operation
objconv copied to clipboard

Support `json.RawMessage` and `json.Decoder.UseNumber`

Open abraithwaite opened this issue 7 years ago • 2 comments

Going along with #12, it seems logical to support these as well to make a drop-in replacement for encoding/json. I'll admit I'm not sure what the implications of each is but they're exported by the standard library so we'd have to find workarounds if they're not in objconv as well.

abraithwaite avatar Aug 08 '17 20:08 abraithwaite

UseNumber is tricky because the decoder is generic and supports multiple serialization formats, for text-based formats like JSON and YAML it may make sense, but for binary encodings it may actually lose precision to have a string representation of a floating point value for example.

RawMessage is kind of the same, it needs to make sense in the context of the multi-format support of objconv, couple of reasons:

  • objconv.RawMessage loses information about the encoding of the byte slice
  • <format>.RawMessage means duplicating types in each sub-package, designing some ways to decode this in a generic way is quite complex
  • I'm not sure what the right behavior is if you try to write a raw message in one format to another encoding (for example json.RawMessage into a msgpack.Encoder), should we error? should we decode it and re-encode it on the fly?
  • On the decoder's performance, RawMessage is a pretty bad API, it requires parsing the content multiple times and adds dynamic memory allocations

In my experience, the need for those APIs is very rare, and can be avoided most of the time by designing code to embrace the zero-copy and strong typing model that objconv encourages.

I'll be happy to take a look at concrete examples that absolutely require those APIs to exist.

achille-roussel avatar Aug 08 '17 20:08 achille-roussel

I don't think the RawMessage and UseNumber features are absolutely necessary in the context I'm viewing them.

I'm seeing what we can do to convert them.

abraithwaite avatar Aug 08 '17 20:08 abraithwaite