-
Notifications
You must be signed in to change notification settings - Fork 269
Heaps #155
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
Conversation
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.
Splice worst case should be O(n) . Instead of splice , pop will work in O(1).
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.
Can you remove this line because it will never execute and it will increase code coverage.
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.
ok
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.
splice will work here in O(1) because heap size is 1 but you can use pop.
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.
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.
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?
Jest Coverage