Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

arosien
Copy link

@arosien arosien commented Apr 19, 2019

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].

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]`.
@stanch
Copy link
Owner

stanch commented May 15, 2019

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”.

@arosien
Copy link
Author

arosien commented May 16, 2019

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.

@stanch
Copy link
Owner

stanch commented May 20, 2019

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 :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants