Skip to content

Increase code coverage #142

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

Merged
merged 16 commits into from
Oct 27, 2019
Merged

Increase code coverage #142

merged 16 commits into from
Oct 27, 2019

Conversation

TheSTL
Copy link
Member

@TheSTL TheSTL commented Oct 25, 2019

🍰 What's Inside?

  • Fix eslint error.
  • Wrong file name corrected.
    • src/DataStructures/Trees/BinarySearchTree/bst-deletion.test.js → src/DataStructures/Trees/BinarySearchTree/height-of-bst/height-of-bst.test.js
    • src/DataStructures/Trees/BinarySearchTree/BinarySearchTree.test.js → src/DataStructures/Trees/BinarySearchTree/bst-remove.test.js
  • Add unit test for BST to increase code coverage.
    • bst-search.test.js
    • bst-minimum.test.js
    • bst-maximum.test.js
    • bst-isEmpty.test.js
  • Fix BST remove function. Earlier if we want to remove root and root has only one child then remove function not working correctly.
  • Increase code coverage for following algo to near 100 %
    - TernarySearch
    - JumpSearch
    - get-smallest-common-number
    - anagrams
    - AStar
    - lowest-common-ancestor
    - height-of-bst
    - postfix-expression-evaluation
    - Queue
    - LinkedList
  • Add Unit test case for DoublyLinkedList because code coverage is decreased when testing LRUCache testing which is implemented on DoublyLinkedList.
  • Create seprate file for Node of DoublyLinkedList

🎉 New Coverage will be 97% after merging.

@TheSTL TheSTL requested a review from ashokdey October 25, 2019 16:38
@TheSTL TheSTL self-assigned this Oct 25, 2019
@TheSTL TheSTL removed the under work label Oct 25, 2019
@TheSTL TheSTL changed the title Code coverage Increase code coverage Oct 26, 2019
Copy link
Member

@ashokdey ashokdey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please make the changes, there are a lot but if you do it file by file that will be easy

@@ -0,0 +1,40 @@
const DLL = require('.');

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Check for DLL should be a Class

expect(queue.dequeue()).toEqual(null);
});

it('Length of linkedlist', () => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

'Length of linkedlist' should be 'Size of Queue'

expect(queue2.length()).toEqual(4);
});

it('Destroy linkedList', () => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

'Destroy linkedList' should be 'Destroy Queue'

describe('Doubly Linked List', () => {
const doublyLinkedList = new DLL();

it('create DLL', () => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

'create DLL' -> 'It should create a DLL'

It's all about Behavior Driven Testing, hence it starts with or contains should

expect(doublyLinkedList.length()).toEqual(0);
});

it('addAtBeginning', () => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change title

@@ -3,18 +3,15 @@
*
* Given values of two values n1 and n2 in a Binary Search Tree, find the Lowest Common Ancestor (LCA). You may assume that both the values exist in the tree.
*/

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Write a meaningful function name

function lca(node, n1, n2) {
if (node == null)
return null;
if (node == null) return null;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use ===

@@ -5,8 +5,7 @@ const BinarySearchTree = require('../index');
// {"left":{"left":{"data":4},"right":{"left":{"data":10},"right":{"data":14},"data":12},"data":8},"right":{"data":22},"data":20}

describe('LCA', () => {

let bst = new BinarySearchTree(20);
const bst = new BinarySearchTree(20);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct all the titles

Comment on lines 6 to 11
bst.add(4);
bst.add(9);
bst.add(2);
bst.add(5);
bst.add(8);
bst.add(12);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use loop here

Comment on lines 17 to 23
bst.remove(6);
bst.remove(4);
bst.remove(9);
bst.remove(2);
bst.remove(5);
bst.remove(8);
bst.remove(12);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use loop here

@TheSTL
Copy link
Member Author

TheSTL commented Oct 26, 2019

Thanks @ashokdey . Please review the changes.

Copy link
Member

@ashokdey ashokdey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great Job 🎉

@ashokdey ashokdey merged commit a37062e into master Oct 27, 2019
@ashokdey ashokdey deleted the codeCoverage branch October 27, 2019 05:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants