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

Consider using jheaps #217

Open
jbduncan opened this issue Aug 3, 2018 · 4 comments
Open

Consider using jheaps #217

jbduncan opened this issue Aug 3, 2018 · 4 comments

Comments

@jbduncan
Copy link
Contributor

jbduncan commented Aug 3, 2018

I was interested by a recent discussion on JGraphT (jgrapht/jgrapht#645) where they discovered that for a certain algorithm, a different sort of heap performed better than the pre-existing one, which lead to a decision to use a heaps data structure library called jheaps.

Should we consider importing jheaps for our own purposes? And should we consider deprecating MapBinaryHeap in the process?

@jrtom
Copy link
Owner

jrtom commented Aug 3, 2018

It's a reasonable question. MapBinaryHeap, however, is a specialized data structure that (as the name implies) combines two different data structures so as to enable operations that aren't part of the heap vocabulary. So while it's possible that we might modify it to use jheaps, we're unlikely to get rid of it entirely.

@jbduncan
Copy link
Contributor Author

jbduncan commented Aug 3, 2018

Ah yes, I remember poking around the source code for MapBinaryHeap a few months ago, realising that it implemented Queue but had unique method(s) not on the interface.

If it were me, I'd keep this issue open until jheaps is needed, but I don't know about you. WDYT?

@jrtom
Copy link
Owner

jrtom commented Aug 3, 2018

It's an internal implementation detail IIRC--that is, MBH itself isn't a part of our APIs, although I think it's a public class--so we can certainly swap our our implementation if that seems warranted. I don't mind leaving this open for now; leave a comment if you want to dig into it further and give jheap a test drive. I don't consider this an urgent issue, though.

@jbduncan
Copy link
Contributor Author

jbduncan commented Aug 3, 2018

I don't mind leaving this open for now; leave a comment if you want to dig into it further and give jheap a test drive. I don't consider this an urgent issue, though.

Acknowledged!

I'll leave this issue to one side for now. I'm currently doing stuff for Spotless, but once that's done, I'll consider having a go at one of the other issues here. :)

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