etaty / rediscala

Non-blocking, Reactive Redis driver for Scala (with Sentinel support)
Apache License 2.0
790 stars 142 forks source link

Some ByteStringSerializer's don't have a corresponding ByteStringDeserializer #135

Open francisdb opened 8 years ago

francisdb commented 8 years ago

Is there a reason that for example ByteStringDeserializer[Long] is missing?

etaty commented 8 years ago

Yes, because you can't write a ByteStringDeserializer[Long] for any ByteString.

francisdb commented 8 years ago

So why do you have a ByteStringDeserializer[Double] then, that one also does not work for any ByteString?

etaty commented 8 years ago

Hum I see, you are right. DoubleRedis should/could be hidden? (It should be hidden as it is Redis specific) https://github.com/etaty/rediscala/blob/master/src/main/scala/redis/Converter.scala#L183-L193

francisdb commented 8 years ago

Honestly, I think you should just have deserialisers for all of the serialisers. Even if they might throw exceptions. Users can always opt for the String version if they want to be safe. Are you sure the RedisDouble is Redis specific? As far as I know there is no such thing as double in Redis. It's just parsing the jave Double#toString() output.