Use putUnencodedChars for Funneling strings.#729
Conversation
As per the warning on [PrimitiveSink.putString](https://javadoc.io/doc/com.google.guava/guava/latest/com/google/common/hash/PrimitiveSink.html#putString(java.lang.CharSequence,java.nio.charset.Charset)) JavaDoc: > Warning: This method, which reencodes the input before processing it, is useful only for cross-language compatibility. For other use cases, prefer putUnencodedChars(java.lang.CharSequence), which is faster, produces the same output across Java releases, and processes every char in the input, even if some are invalid. Or maybe leave this out and let people make their own decision as to what they want for the `Funnel[String]` instance?
|
|
||
| implicit val booleanFunnel: Funnel[Boolean] = funnel[Boolean](_.putBoolean(_)) | ||
| implicit val stringFunnel: Funnel[String] = funnel[String](_.putString(_, Charsets.UTF_8)) | ||
| implicit val stringFunnel: Funnel[String] = Funnels.unencodedCharsFunnel.asInstanceOf[Funnel[String]] |
There was a problem hiding this comment.
Guess instead of using the cast, this could also be funnel[String](_.putUnencodedChars(_))
There was a problem hiding this comment.
If we wrote our own typeclass for this (that was equivalent to Funnel but defined in Scala), we could use Scala's SAM support to define instances, like implicit val stringFunnel: OurFunnel[String] = _ putUnencodedChars _, add methods to it like contramap (which is what the by method in here seems to be?), widen, add a variance annotation to it, etc... which could then be converted to a Guava Funnel at use-site?
There was a problem hiding this comment.
Since Funnel is invariant in the type param, maybe we could define helpers like other libraries have, e.g.
def narrowFunnel[AA, A <: AA](funnel: Funnel[AA]): Funnel[A] = funnel.asInstanceOf[Funnel[A]]so then stringFunnel could be defined as:
implicit val stringFunnel: Funnel[String] = narrowFunnel(Funnels.unencodedCharsFunnel)?
And maybe even something to not need the cast on the "unboxed" funnels like Int, Long, etc...
There was a problem hiding this comment.
I think a simple solution would be to define a single implicit def for all CharSequence instances
implicit def charSequenceFunnel[T <: CharSequence]: Funnel[T] =
Funnels.unencodedCharsFunnel().asInstanceOf[Funnel[T]]
As per the warning on PrimitiveSink.putString JavaDoc:
Or maybe leave this out and let people make their own decision as to what they want for the
Funnel[String]instance?