Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ADT subtype codec is incompatible with ADT codec #17

Closed
mushtaq opened this issue May 8, 2019 · 3 comments
Closed

ADT subtype codec is incompatible with ADT codec #17

mushtaq opened this issue May 8, 2019 · 3 comments

Comments

@mushtaq
Copy link
Contributor

mushtaq commented May 8, 2019

Test

sealed trait Animal
case class Tiger(name: String) extends Animal

object Animal {
  import io.bullet.borer.derivation.MapBasedCodecs._
  implicit val tigerCodec: Codec[Tiger] = deriveCodec[Tiger]
  implicit val animalCodec: Codec[Animal] = deriveCodec[Animal]
}

val tiger     = Tiger("king")
val byteArray = Cbor.encode(tiger).toByteArray
Cbor.decode(byteArray).to[Animal].value

Exception

io.bullet.borer.Borer$Error$UnexpectedDataItem: 
Unexpected data item: 
Expected [Array with 2 elements for decoding an instance of type [csw.params.core.formats.Animal]] but got [Map Header (1)]

Workaround

object Animal {
  import io.bullet.borer.derivation.MapBasedCodecs._
  implicit lazy val animalCodec: Codec[Animal] = {
    implicit val tigerCodec: Codec[Tiger] = deriveCodec[Tiger]
    deriveCodec[Animal]
  }
  implicit lazy val tigerCodec: Codec[Tiger] = Codec(
    animalCodec.encoder.compose(identity),
    animalCodec.decoder.map(_.asInstanceOf[Tiger])
  )
}
@sirthias
Copy link
Owner

sirthias commented May 8, 2019

Well, I'm unsure whether and ADT subtype codec should be compatible with the ADT codec.
Codec[T] as well as Encoder[T] and Decoder[T] are invariant in their type parameters. So a Codec[Tiger] is not a Codec[Animal].
What's your use case?

The reason for the error you are seeing in your snippet above is that, in release 0.8.0, ADTs are encoded as a two element array, with the type-id being the first element and the result of the subtype encoder being the second element.
So the ADT encoder adds it's own wrapping layer around the subtype codec to augment the encoding with the type information.

With map-based codecs this could be done in a way that makes the ADT encoding compatible with the subtype encoding, by adding a "special" map member (like $typeId or the like) to the encoded map of the subtype, but I'd rather not go down this route, because of the hacky nature of this approach and because it wouldn't work with subtype encoders that don't produce a map.

NOTE: In 0.9.0 the ADT encoding will change from a 2-element array to a single-element map, with the typeid becoming the map key and the subtype encoding becoming this key's map value.

@mushtaq
Copy link
Contributor Author

mushtaq commented May 8, 2019

I should not have used the word incompatible, as you explain the codecs are invariant. It just felt wrong that Tiger is being encoded slightly differently based on its static type within an ADT.

In comparison, saw the upickle docs (see IntOrTuple example) where they always produce the typehint if a type is part of an ADT and it felt like a right choice.

Either Array or Map based encoding for the ADT is fine for me.

@sirthias
Copy link
Owner

sirthias commented May 8, 2019

Yes, @mushtaq, thank you for the pointer to upickle.
I completely get your intuition...
However, even upickle would encode an Animal differently from a Tiger. It's just that the Decoder[Tiger] would ignore the additional $type member.

However, IMHO this is just "accidental", because a map-based encoding does indeed allow for detecting superfluous members and simply skipping them.
This is not the case for array-based encoders, which depend on the number of arguments exactly matching.

So, IMHO the cleanest solution is to simply make clear (in the docs as well) that an Encoder[Animal] produces output thats different from the Encoder[Tiger] for the same object and that the Decoder type has to match the one used for encoding. Just as the type variance in fact describes it.

@sirthias sirthias closed this as completed May 8, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants