Skip to content
This repository has been archived by the owner on May 15, 2019. It is now read-only.

Zipper.unselect does not preserve order of flatMapped children #36

Closed
josharnold52 opened this issue Jul 27, 2011 · 2 comments
Closed

Comments

@josharnold52
Copy link
Contributor

If Zipper.flatMap is used to replace one node with many, then Zipper.unselect does not preserve the order of the many. This is because a Map[Int,Set[Int]] is used to correlate elements from the original group to elements in the modified group, and Set makes no ordering guarantees.

For Zipper as it stands, I think the elements of that set will always form a contiguous range, so a Set might actually be overkill here. I'm going to try replacing it with a Range or similar and see if that holds up. Assuming that works, do you envision ever needing the more general correlation provided by Set?

scala> val tree = <top><a /></top>.convert
tree: com.codecommit.antixml.Elem = <top><a/></top>

scala> val expand = (tree \ 'a) flatMap {
     |   case e:Elem =>
     |   for(indx <- 0 until 10) yield e.copy(name=e.name+indx)  
     | }
expand: com.codecommit.antixml.Zipper[com.codecommit.antixml.Elem] = <a0/><a1/><a2/><a3/><a4/><a5/><a6/><a7/><a8/><a9/>

scala> val newTree = expand.unselect
newTree: com.codecommit.antixml.Zipper[com.codecommit.antixml.Node] = <top><a0/><a5/><a1/><a6/><a9/><a2/><a7/><a3/><a8/><a4/></top>
josharnold52 added a commit to josharnold52/anti-xml that referenced this issue Jul 27, 2011
@josharnold52
Copy link
Contributor Author

The above commit (pull request #37 ) addresses the issue. I was actually able to replace the Set[Int] with just an Int defining the # of nodes in the Zipper corresponding to the source child group. That suffices to reconstruct the correspondence if you make 2 assumptions:

  1. Every node in the Zipper corresponds to a single node in the source.
  2. If a N1 precedes N2 in the source, then the Zipper's nodes corresponding to N1 precede the Zipper's nodes corresponding to N2.

These assumptions already held true for all Zipper instances, so nothing is really changing in terms of functionality. It's just that now those assumptions are required for ZContext to make sense, whereas before it allowed for more general mappings.

@djspiewak
Copy link
Owner

I'm so sorry it's taken me so long to get back to you on this. Basically, the new restriction is that the mapping end-points need to be contiguous for a particular element? I think that's a fairly safe assumption. In fact, it falls precisely within the constraints of normal sequence operations (map, flatMap, etc). I'm going to pull in your changes; thanks!

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

No branches or pull requests

2 participants