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

wrong term/implementation in Data.Heap.Binary and Data.Tree.Splay #1

Open
MaskRay opened this issue Jan 14, 2012 · 1 comment
Open

Comments

@MaskRay
Copy link

MaskRay commented Jan 14, 2012

I have taken a look at your project roughly and found something weird. The Data.Heap.Binary you implemented did not seem to be the Binary heap' I learned from books. Itsmerge' operation looked similar to Leftist tree and size balanced tree.

And in Data.Tree.Splay, you seemed to provide a top-down implementation of Splay trees, however, the lack of double rotates may not guarantee the O(log n) amortized time complexity.

Regards,
Ray Song

@bhickey
Copy link
Owner

bhickey commented Jan 16, 2012

It is size balanced. Insertion prefers the shallower subtree. The heap implementation you'll find in most textbooks probably involves inserting the new node at a root and bubbling it up. Since I don't have O(1) access to a leaf, I push down from the root. Barring some egregious mistake on my part, this should work.

If you want to submit a failing test, I'll be happy to patch it.

I've been thinking about reassessing the whole implementation. Enforcing elements that are instances of Ord forces the use of adapter classes in some cases. Yuck!

Brendan

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

No branches or pull requests

2 participants