buildo / ingredients

MIT License
13 stars 2 forks source link

Add ordering for case enum #28

Closed andreaferretti closed 8 years ago

andreaferretti commented 8 years ago

case enum would be much more useful if there was a way to convert to and from Int, based on the ordering. This would give significant space savings in serialization protocols

utaal commented 8 years ago

Hi Andrea, thanks for the feedback. Do you have a specific use case in mind you can share? I think there are a couple of different ways to add this in with minimal impact on the footprint of the library (keeping it simple and small is one of the main goals) and knowing where this could be useful would help in selecting a design.

andreaferretti commented 8 years ago

Well, I need to serialize enums, saving space where possible. If enums are declared using Scala Enumeration I can easily convert back and forth from an integer using _.id and MyEnum(_). This allows me to use one or a few bytes in serialization protocols.

With your approach I can convert back and forth from strings, but these can take much more space.

What would be useful is a pair of functions, say Planet => Int and Int => Planet, such that Earth maps to 0, Venus to 1 and so on.

Please, let me know if this is clear enough, or if I need to explain this better.

utaal commented 8 years ago

Right, yep, I was looking for a concrete use case (and serialization format) which I understand you may be unable to share. The pair of functions can naturally be provided as a typeclass - what I am concerned about is ensuring compatibility when changes happen to the serial protocol: e.g. what should happen if an enumeration member is added in between the existing one? Maybe the integer values should be explicitly assigned by the user. I understand that scala Enumeration assigns integer values from 0 upwards but I'm not necessarily a fan of that approach for serialization due to possible mismatch when the serialized value is written and read by two different versions of the program.

I'm considering adding support for a single abstract member:

sealed abstract trait Planet { val id: Int }
case object Mercury extends Planet { val id = 0 }
case object Venus extends Planet { val id = 3 }

may be written as something like

@enum[Int] trait Planet {
  object Mercury = 0
  object Venus = 3
}

and the Int => Planet conversion can be (automatically) provided in a typeclass, thus:

scala> Planet.Mercury.id
res0: 0: Int
scala> Planet.fromId(3)
res1: Some(Planet.Venus): Option[Planet]

What do you think?

andreaferretti commented 8 years ago

Yeah, that would be fine. About serialization protocols, it is not that I cannot share it, it is that it can change, and the requirement stays the same. If it helps make it concrete, take BooPickle, especially the transformPickler that generates a pickler for A given a pickler for B, if A and B can be put in a bijection. But a similar concern would hold for MsgPack or whatever.

About things breaking when the data model changes - sometimes it is an acceptable tradeoff. For instance, BooPickle essentially does not allow data changes - in exchange you get a very fast (de)serializer.

utaal commented 8 years ago

Here's a first attempt: https://github.com/buildo/ingredients/pull/29/files What do you think? In the test you can find an example of how the typeclass can be used to provide instances for a (un)pickler.

And an instance for something like transformPickler in BooPickle can be made available via:

implicit def caseEnumValueTransformPickler[T <: ValuedCaseEnum](
    implicit val instance: CaseEnumValue[T]): TransformPickler[T, T#Value] = 
        transformPickler[T, T#Value](instance.caseToValue _, x => instance.caseFromValue(x).get)

There's no macro for concise syntax yet: I'll give it a try tomorrow (it shouldn't be particularly surprising).

andreaferretti commented 8 years ago

Yeah, it seems to work perfectly! Thanks :-)

utaal commented 8 years ago

Good news! Got the macro implemented and cleaned up the branch (renamed the confusing ValuedEnum to IndexedEnum). I'll put it through review, it should be on bintray in a few days.

Edit: feel free to comment on https://github.com/buildo/ingredients/pull/29

andreaferretti commented 8 years ago

Great, thanks

utaal commented 8 years ago

@andreaferretti 0.2.0 is out with indexedEnum and additional documentation in the readme