gnieh/diffson

Split JsonFormat into Encoder and Decoder (or Writer and Reader)

Closed this issue · 2 comments

Currently diffson requires combined JsonFormat for library integration, which is not be supported by some libraries such as circe by design. Here's the corresponding section in it's design document:

The public API should not contain unnecessary types. Argonaut (like many other great serialization libraries) includes a Codec type class that combines Encoder and Decoder, but it's only really useful for definitions: it allows you to define an encoder-decoder pair together, but its generally not useful as a type constraint, since many types with both Encoder and Decoder instances will not have a Codec instance. I personally find this confusing, and would prefer to keep convenience-oriented types like this out of the API.

What this means concretely: There's unlikely to be a Codec in circe until we come up with a clean way to make it useful as a type constraint (although if demand gets loud enough I would be willing to consider compromise here).

Although it's possible to create a proxy class that combines circe's Encoder and Decoder, I'd suggest that we separate them in diffson for more flexibility.

Hi,

I am not convinced with the arguments in your citation in general. For me it really depends on the use cases. I will review the diffson usage of Format to see if diffson always needs both encoder and decoder. If so, I don’t see it really likely for diffson to adopt this approach. Diffson is not a json library but a concrete usage of such libraries.

I do not see either what kind of flexibility it would bring in our use case. Can you please explain in more details what you mean with "more flexibility"?

It just means adding circe support as it is, without introducing unnecessary proxy layer. I have CirceDiffsonInstance which defines a JsonFormat like this:

  trait JsonFormat[A] {
    def decode(c: HCursor): Result[A]

    def encode(a: A): Json
  }
  object JsonFormat {
    implicit def apply[A](implicit encoder: Lazy[Encoder[A]], decoder: Lazy[Decoder[A]]): JsonFormat[A] =
      new JsonFormat[A] {
        override def decode(c: HCursor): Result[A] = decoder.value.apply(c)

        override def encode(a: A): Json = encoder.value.apply(a)
      }
  }

Although it just works, I feel a bit uncomfortable about this because it's against circe's philosophy and it adds runtime overhead, even though it's negligible.

Looking at JsonSupport#JsonProvider, type Marshalling[T] is used in three places; marshal, unmarshal, and patchMarshaller. IMHO it's natural to split Marshalling into Marshall and Unmarshall to correspond to each function and split patchMarshaller into patchMarshaller and patchUnmarshaller. Thanks to diffson's typeclass oriented interface, it'll introduce source level breakage of zero or few lines to end users.

Being more specific, JsonDiff#diff only requires Marshall and JsonPatch#apply requires the both. For the latter, just changing the type parameter from [T: Marshalling] to [T : Marshall : Unmarshall] is enough.

If a JSON library provides its reader/writes only in the combined form, Marshall and Unmarshall can just point the same type so that there's no problem. And there's no one taking this approach AFAIK and every typeclass based JSON library already exposes separate traits regarding each case.