harttle / contest.js

Ready for contest use! Data structures and algorithms in pure JavaScript with zero dependency.
http://harttle.land/contest.js/
MIT License
41 stars 9 forks source link

Can we replace the Heap compare function to Array.sort's? #10

Closed Jozdortraz closed 2 years ago

Jozdortraz commented 3 years ago

Heap compare function: cmp = (lhs, rhs) => lhs < rhs Array.sort compare function: cmp = (lhs, rhs) => lhs - rhs

It will get user confused when using Array.sort and Heap together.

e.g. LeetCode 1851

    queries = queries.map((e, i) => [e, i]).sort((a, b) => a[0] - b[0]);
    intervals.sort((a, b) => a[0] - b[0]);

    const pq = new Heap((a, b) => a[1] - a[0] < b[1] - b[0]);
harttle commented 3 years ago

Unfortunately it's changed to a boolean function from Array compare intentionally. It's consistent with cpp STL.

Actually, sort compare and Heap compare in STL is different. I think it makes sense but I'm open for this topic. Try post example from other languages or just vote by post your opinion on this thread. I will keep this issue open.

upupming commented 3 years ago

@harttle We can add a typeof to check the return type of the compare function to make it capatible with both style. But the template will be much longer.

Anthor issue is that STL priority queue is max heap by default, but our heap is min heap by default. What do you think, should we keep consistant with STL?

harttle commented 2 years ago

@upupming please find info below.

make it capatible with both style

I'd rather keep it simple and provide only one way to use contest.js, I believe it will eventually be proven easier for users.

but our heap is min heap by default

You're right! Maybe I find it more convenient to be min heap once upon a time. But I agree with you it's inconsistent and we should make it a max heap.

harttle commented 2 years ago

I decided to use Array#sort's compare for all of the classes, including Heap, TreeSet, TreeMultiSet, RBTree. Merged to master in this commit: https://github.com/harttle/contest.js/commit/2152fb1358bae62b63a1445f121e5411c9cd73c8