search

GlenKPeterson / Paguro

Generic, Null-safe, Immutable Collections and Functional Transformations for the JVM
Other
312 stars 24 forks source link

New failing tests proposed fix #33

Open jafingerhut opened 5 years ago

jafingerhut commented 5 years ago

This PR combines both the new failing tests in PR https://github.com/GlenKPeterson/Paguro/pull/32 and also has some proposed fixes.

You should of course treat these proposed fixes with suspicion, since I am not as deeply versed in the code as you are.

The basic idea is that after a join (and I believe also a split), both mutable and immutable RrbtTree data structures have an empty focus. The append method was not handling the case of an empty focus correctly. The proposed fix makes an explicit extra condition check for an empty focus, and handles it. There is no need to push an empty focus, but we should create a new focus with one element in it.

GlenKPeterson commented 5 years ago

This could be the right fix. I worried I might have led you astray with my suggestion to look in the join method. Just thinking, not looking at my code, I'm guessing that after the join, the focusStartIndex is pointing to the start (left-hand side) of the tree. Append needs to make sure that if there is something in the focus, it gets pushed to the appropriate place, then sets focusStartIndex to the right-most side of the tree and adds stuff to the focus. Append and concat both need that behavior.

My new theory is that you can cause this bug to occur without calling join. Is that where you're headed too?

Sorry, my computer time is up for today.

jafingerhut commented 5 years ago

I am pretty sure it could be caused by calling split followed by append, too, since I believe split also leaves the focus empty.

I do not know whether it can be caused in a sequence that calls neither split nor join, but don't know enough about the complete code base to say yet.