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

New failing tests proposed fix #33

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

jafingerhut
Copy link
Contributor

No description provided.

@jafingerhut
Copy link
Contributor Author

This PR combines both the new failing tests in PR #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
Copy link
Owner

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
Copy link
Contributor Author

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.

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