zipper icon indicating copy to clipboard operation
zipper copied to clipboard

Delay passing Unzip[A] to Zipper until it is really needed.

Open arosien opened this issue 5 years ago • 3 comments

This makes it possible to write a cats.Functor instance giving us a map method. With the Unzip[A] passed to the contructor, we can't write map without the "target" Unzip[B].

arosien avatar Apr 19 '19 19:04 arosien

Sorry for the delay! I am not familiar with cat.Functor, could you give an example of how this change helps? My concern is that it allows to pass an unzipping implementation that is not consistent with the state of the zipper. Also if an API receives a zipper, I think it’s reasonable to expect that the zipper “knows what to do”.

stanch avatar May 15 '19 12:05 stanch

My real goal is to be able to map a Zipper, so this might be an alternative PR:

 def map[B](f: A => B)(implicit ub: Unzip[B]): Zipper[B] =
    copy(left = left.map(f), focus = f(focus), right = right.map(f), top = top.map(_.map(f))) 

The issue with cats.Functor is secondary. It requires a map function whose only input is the A => B function, but if the Zipper constructor requires a Unzip[B], there's no way to pass it in this case.

arosien avatar May 16 '19 17:05 arosien

This is interesting, because I believe your map implementation assumes that up to the mapping point, B would have been zipped/unzipped the same way as A. Which leads to the idea that if you have a mapping between A and B, you can actually derive Unzip[B] in terms of Unzip[A].

Here is the definition of Unzip:

trait Unzip[A] {
  def unzip(node: A): List[A]
  def zip(node: A, children: List[A]): A
}

Upon closer inspection, A => B is not enough to derive Unzip[B]. However if you had both A => B and B => A, that would be possible:

trait Unzip[A] { self =>
  def unzip(node: A): List[A]
  def zip(node: A, children: List[A]): A

  final def imap[B](f: A => B)(g: B => A) = new Unzip[B] {
    def unzip(node: B): List[B] = self.unzip(g(node)).map(f)
    def zip(node: B, children: List[B]): B = f(self.zip(g(node), children.map(g)))
  }
}

So technically I think this makes Zipper an invariant functor. Happy to merge such a PR :)

stanch avatar May 20 '19 09:05 stanch