Heaps #155
Heaps #155
Conversation
| if (this.heap.length > 1) { | ||
| // move the leaf to the root | ||
| this.heap[0] = this.heap[this.heap.length - 1]; | ||
| this.heap.splice(this.heap.length - 1, 1); |
There was a problem hiding this comment.
Splice worst case should be O(n) . Instead of splice , pop will work in O(1).
| return max; | ||
| } | ||
|
|
||
| return max; |
There was a problem hiding this comment.
Can you remove this line because it will never execute and it will increase code coverage.
| } | ||
|
|
||
| if (this.heap.length === 1) { | ||
| this.heap.splice(this.heap.length - 1, 1); |
There was a problem hiding this comment.
splice will work here in O(1) because heap size is 1 but you can use pop.
There was a problem hiding this comment.
Changes required:
-
Use pop instead of splice.
-
To increase code coverage you can add this test case
`
[1, 34, 43, 54, 123].forEach(el => mh.add(el));mh.remove();
mh.remove();
mh.remove();
mh.remove();
mh.remove();
mh.remove();
expect(mh.getMax()).toEqual(null);
`
because it will covered, uncovered line 42, and uncovered branch 19,30.
Code coverage will be 100% after this.
Why not you add this after merging this branch? |
|
@ashokdey ok |
🏖 What's Inside?