bizzabo / play-json-extensions

+22 field case class formatter and more for play-json
http://cvogt.org/play-json-extensions/api/
Other
195 stars 47 forks source link

Added support for hinted formats #22

Closed alkersan closed 5 years ago

alkersan commented 8 years ago

Hi Christopher! Please, review this proposal of possible solution for #18. I've borrowed some terminology from json4s's polymorphic serialization and added some possibility to customize shape of type hints.

cvogt commented 8 years ago

Interesting. @n8than you guys doing something like this as well?

The way we do this exact thing without any extension to play-json-extensions is:

sealed trait ApiRequest
case class Withdraw(amount: BigDecimal, _type: Withdraw.type = Withdraw) extends ApiRequest
case class Deposit(amount: BigDecimal, _type: Deposit.type = Deposit) extends ApiRequest

formatSingleton takes are of the rest. That means serialization specifics leak into the Scala interfaces, but in a way that's a good thing, because it makes things clearer. WDYT @alkersan ?

alkersan commented 8 years ago

@cvogt this is of cause nice little trick, but i think that making hinting process more explicit has it's benefits. Take for example formatSealed. Currently it iterates though all members (in worst case) with orElse until hits correct format. Wouldn't it be more efficient with hinted formats to pattern match on hint and use exact format directly without attempting all others? Probably on small hierarchies performance penalty isn't noticeable, but it grows linearly with size of hierarchy and complexity of parsing each subtype. This is what I tried to achieve with formatHintedSealed

alkersan commented 8 years ago

@cvogt sorry, i'm haven't really understood how verifyKnownDirectSubclassesPostTyper works under the hood, so I've just copied usage from formatSealed. Hope it's correct. I also have doubts about Tags. Maybe it should be name like Tagging or TaggingStrategy ?

cvogt commented 8 years ago

TagStrategy reminds me of http://projects.haykranen.nl/java/

How about TagStyle ;)? But TagStrategy works as well, no objection.

cvogt commented 8 years ago

@alkersan sorry if we may have miscommunicated. I liked that formatTagged is supposed to be used together with formatSealed. No need for formatSealedTagged. How about we get rid of it and get this merged :)?

alkersan commented 8 years ago

@cvogt sorry for delay, I was kind of away from coding last few days. I'll do cleanups and push update soon

alkersan commented 8 years ago

Please review once again macro implementation:

alkersan commented 8 years ago

@cvogt did you have chance to look at latest revision of this pr?

caente commented 5 years ago

@cvogt @alkersan is this still relevant?

alkersan commented 5 years ago

Didn't use play-json for few years. I think this can closed