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
Use Priority queue in chunk graph algorithm #8238
Conversation
For maintainers only:
|
Thank you for your pull request! The most important CI builds succeeded, we’ll review the pull request soon. |
this.heap = []; | ||
/** @type {function(T):number}*/ | ||
this.priorityFn = priorityFn; | ||
for (let item of items) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since items
is optional, you must use an if
to guard the iteration
} | ||
const poppedValue = this.heap.pop(); | ||
this._siftDown(); | ||
return poppedValue.item; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Calling dequeue
on an empty PriorityQueue
would result in a TypeError
. It would be better to change this line to:
return poppedValue.item; | |
return poppedValue ? poppedValue.item : undefined; |
No longer needed because of #8242 |
What kind of change does this PR introduce?
algorithmic performance improvement
Did you add tests for your changes?
existing tests
Does this PR introduce a breaking change?
no
What needs to be documented once your changes are merged?
nothing