Circe: decode multi-level ADT efficiently

Question

I want to decode the following ADT with Circe:

sealed trait PaymentType
object PaymentType extends EnumEncoder[PaymentType] {
  case object DebitCard extends PaymentType
  case object Check     extends PaymentType
  case object Cash      extends PaymentType
  case object Mobile    extends PaymentType
}
sealed trait CreditCard extends PaymentType
object CreditCard extends EnumEncoder[CreditCard] {
  case object UNKNOWN_CREDIT_CARD extends CreditCard
  case object NOT_ACCEPTED        extends CreditCard
  case object VISA                extends CreditCard
  case object MASTER_CARD         extends CreditCard
  case object DINERS_CLUB         extends CreditCard
  case object AMERICAN_EXPRESS    extends CreditCard
  case object DISCOVER_CARD       extends CreditCard
}

As you can see, there is a parent type PaymentType, which has some direct inheritors and another sealed trait family CreditCard. Now decoding is done like this:

object CreditCard {
  implicit val decoder: Decoder[CreditCard] = Decoder.instance[CreditCard] {
  _.as[String].map {
    case "NOT_ACCEPTED"     => NOT_ACCEPTED
    case "VISA"             => VISA
    case "MASTER_CARD"      => MASTER_CARD
    case "DINERS_CLUB"      => DINERS_CLUB
    case "AMERICAN_EXPRESS" => AMERICAN_EXPRESS
    case "DISCOVER_CARD"    => DISCOVER_CARD
    case _                  => UNKNOWN_CREDIT_CARD
  }
}

object PaymentType {
  implicit val decoder: Decoder[PaymentType] = Decoder.instance[PaymentType] {
    _.as[String].flatMap {
      case "DebitCard" => Right(DebitCard)
      case "Check"     => Right(Check)
      case "Cash"      => Right(Cash)
      case "Mobile"    => Right(Mobile)
      case _           => Left(DecodingFailure("", List()))
    }
  }.or(CreditCard.decoder.widen)
}

What I don't like is the PaymentType decoder, particularly the fact that I need to create an extra and unnecessary instance of DecodingFailure in completely normal scenario when one encounters credit card-based payment type. We already spend 99.9% of CPU on JSON processing, and it just doesn't look right. Either it's bad ADT design, or there should be a way in Circe to handle this better. Any ideas?

score 4 · Accepted Answer · answered Feb 17 '17 at 15:15

You can move the fallback to the CreditCard decoder into the PaymentType decoder cases, which lets you avoid having to fail:

implicit val decoder: Decoder[PaymentType] = Decoder.instance[PaymentType] { c =>
  c.as[String].flatMap {
    case "DebitCard" => Right(DebitCard)
    case "Check"     => Right(Check)
    case "Cash"      => Right(Cash)
    case "Mobile"    => Right(Mobile)
    case _           => CreditCard.decoder(c)
  }
}

In a case like this, though, I'd probably factor out the string parsing into separate methods:

sealed trait PaymentType
object PaymentType extends EnumEncoder[PaymentType] {
  case object DebitCard extends PaymentType
  case object Check     extends PaymentType
  case object Cash      extends PaymentType
  case object Mobile    extends PaymentType

  private val nameMapping = List(DebitCard, Check, Cash, Mobile).map(pt =>
    pt.productPrefix -> pt
  ).toMap

  def fromString(input: String): Option[PaymentType] = nameMapping.get(input)
}

sealed trait CreditCard extends PaymentType
object CreditCard extends EnumEncoder[CreditCard] {
  case object UNKNOWN_CREDIT_CARD extends CreditCard
  case object NOT_ACCEPTED        extends CreditCard
  case object VISA                extends CreditCard
  case object MASTER_CARD         extends CreditCard
  case object DINERS_CLUB         extends CreditCard
  case object AMERICAN_EXPRESS    extends CreditCard
  case object DISCOVER_CARD       extends CreditCard

  private val nameMapping = List(
    NOT_ACCEPTED,
    VISA,
    MASTER_CARD,
    DINERS_CLUB,
    AMERICAN_EXPRESS,
    DISCOVER_CARD
  ).map(pt => pt.productPrefix -> pt).toMap

  def fromString(input: String): CreditCard =
    nameMapping.getOrElse(input, UNKNOWN_CREDIT_CARD)
}

Then you can write the decoders in terms of the fromString methods, which just feels like a better way of chopping up the problem to me (off the top of my head I'm not sure which approach will involve fewer allocations). That's probably largely a matter of taste, though.

Yeah, I've thought about falling back directly to `CreditCard` from `PaymentType`. My initial reaction was not to do so, because it makes parent class aware of its children, which is not good practice. But since it's all sealed types, it's probably not that bad. And it's certainly better than currently used alternative. — Haspemulator, Feb 17 '17 at 15:40
@Haspemulator Yeah, since you're already stuck with the `.or(CreditCard.decoder)` I don't see a big difference between the two in that respect. — Travis Brown, Feb 17 '17 at 16:12
Right, what I've already posted contained such reverse coupling anyways. Yet before this I didn't have fallback at all, and it just failed at runtime with `MatchError` when some of the `CreditCard` strings was encountered. — Haspemulator, Feb 17 '17 at 16:22

Circe: decode multi-level ADT efficiently

1 Answers1