twitter / summingbird

Streaming MapReduce with Scalding and Storm
https://twitter.com/summingbird
Apache License 2.0
2.14k stars 267 forks source link

TailProducer clean up #558

Open johnynek opened 10 years ago

johnynek commented 10 years ago

If NamedProducer and AlsoProducer kept the full type of the types they wrapped, and we made TailProducer a typeclass (trait IsTail[P]) we could avoid having the duplication of AlsoTail and Named tail producer with:

case class NamedProducer[P, N <: Producer[P, T], T](named: N) extends Producer[P, T]
case class AlsoProducer[P, Left <: Producer[P, Any], Right <: Producer[P, T], T](ensure: Left, result: Right) extends Producer[P, T]

sealed trait IsTail[T]
object IsTail {
  implicit def summer[P, K, V]: IsTail[Summer[P, K, V]] = new IsTail[Summer[P, K, V]]
  implicit def sink[P, T]: IsTail[Sink[P, T]] = new IsTail[Sink[P, T]]
  implicit def named[P, N, T](implicit t: IsTail[N]): IsTail[NamedProducer[P, N, T]] = ...
  implicit def also[P, L, R, T](implicit r: IsTail[R]): IsTail[AlsoProducer[P, L, R, T]] = ...
} 

then plan becomes:

trait Platform[P] {
  def plan[R <: Producer[P, T], T](p: R)(implicit i: IsTail[R]): Plan[T]
}
xudifsd commented 9 years ago

Wow, this indeed makes it much simpler, but this will cause backwards incompatible, right?

johnynek commented 9 years ago

It would be backwards incompatible for binary, but I am assuming most user code does not reference TailProducer so it could be source compatible.

That said, the gain may be low and not outweigh even a little backwards incompatibility.