Implement Binary Tree to Binary Search Tree Conversion - Solution #127

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

Jump to bottom

Merged

TheSTL merged 2 commits into knaxus:master from k88manish:master

Oct 26, 2019

Merged

Implement Binary Tree to Binary Search Tree Conversion - Solution #127

TheSTL merged 2 commits into knaxus:master from k88manish:master

Oct 26, 2019

Conversation

k88manish

Copy link

Contributor

@k88manish k88manish commented Oct 22, 2019

@ashokdey @TheSTL - Raised a new PR because the last PR was raised with the wrong Author information

@k88manish


 Implement Binary Tree to Binary Search Tree Conversion - Solution

2ba8925

@k88manish k88manish mentioned this pull request

Oct 22, 2019

Implement Binary Tree to Binary Search Tree Conversion #90

Closed

@TheSTL TheSTL requested a review from ashokdey

October 22, 2019 22:14

@TheSTL TheSTL assigned k88manish

Oct 22, 2019

@TheSTL TheSTL added Hacktoberfest under-review labels

Oct 22, 2019

ashokdey

ashokdey requested changes

Oct 23, 2019

View reviewed changes

src/_Problems_/binary-tree-to-binary-search-tree/index.js Outdated

function arrayToBST(arr, root) {

const node = root;

// Base case

if (!node) return;

Copy link

Member

@ashokdey ashokdey Oct 23, 2019

Choose a reason for hiding this comment

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

You have to return null meaning the tree is empty

src/_Problems_/binary-tree-to-binary-search-tree/Node.js Outdated

Comment on lines 1 to 7

module.exports = class Node {

constructor(value) {

this.value = value;

this.leftChild = null; // will be a node

this.rightChild = null; // will be a node

}

};

Copy link

Member

@ashokdey ashokdey Oct 23, 2019

Choose a reason for hiding this comment

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

You can re-use the Node class already present in the codebase

src/_Problems_/binary-tree-to-binary-search-tree/index.js

Comment on lines +20 to +36

function storeInorder(root) {

/** left - root - right */

if (root === null) return [];

// First store the left subtree

let arr = [];

const left = storeInorder(root.leftChild);

arr = [...left, ...arr];

// Append root's data

arr = [...arr, root.value];

// Store right subtree

const right = storeInorder(root.rightChild);

arr = [...arr, ...right];

return arr;

}

Copy link

Member

@ashokdey ashokdey Oct 23, 2019

Choose a reason for hiding this comment

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

I think a better way to do this is to create a file inside Problems/tree-traversal directory nd have the following:

inorderTraversal(root) : returns array of elements
preorderTraversal(root) : returns array of elements
so on...

And re-use the function here.

Alternatively, I am going forward to create these as helpers inside DataStructures/Trees/traversal so that they can be re-used

src/_Problems_/binary-tree-to-binary-search-tree/index.js

}

// Helper function to copy elements from sorted array to make BST while keeping same structure

function arrayToBST(arr, root) {

Copy link

Member

@ashokdey ashokdey Oct 23, 2019

Choose a reason for hiding this comment

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

Can you mention the runtime complexity of this function?

src/_Problems_/binary-tree-to-binary-search-tree/index.js Outdated

function binaryTreeToBST(root) {

// Tree is empty

if (!root) return;

Copy link

Member

@ashokdey ashokdey Oct 23, 2019

Choose a reason for hiding this comment

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

return null

src/_Problems_/binary-tree-to-binary-search-tree/index.js Outdated

if (!root) return;

const arr = storeInorder(root);

arr.sort((a, b) => a - b);

arrayToBST(arr, root);

Copy link

Member

@ashokdey ashokdey Oct 23, 2019

Choose a reason for hiding this comment

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

@TheSTL, shouldn't it return a new BST?

Copy link

Member

@TheSTL TheSTL Oct 23, 2019

Choose a reason for hiding this comment

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

@ashokdey Yes, it should return the root node of new created BST.

@ashokdey ashokdey requested a review from TheSTL

October 23, 2019 18:38

@ashokdey ashokdey added the changes-requested label

Oct 23, 2019

TheSTL

TheSTL reviewed

Oct 23, 2019

View reviewed changes

src/_Problems_/binary-tree-to-binary-search-tree/binary-tree-to-binary-search-tree.test.js Outdated

Comment on lines 8 to 12

tree = new Node(10);

tree.leftChild = new Node(30);

tree.leftChild.leftChild = new Node(20);

tree.rightChild = new Node(15);

tree.rightChild.rightChild = new Node(5);

Copy link

Member

@TheSTL TheSTL Oct 23, 2019

Choose a reason for hiding this comment

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

Binary Tree implementation is already present.

Copy link

Member

@ashokdey ashokdey Oct 24, 2019

Choose a reason for hiding this comment

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

@k88manish, you can use this Binary Tree using level order to create the tree here.

@k88manish


 Binary Tree to BST

7578fb1

- Return null value
- Using Node Class already existed
- Mentioned Runtime complexity of function
- Return BST root node
- Used BinaryTree implementation to create the tree.

@k88manish

Copy link

Contributor Author

k88manish commented Oct 26, 2019

Hi @ashokdey @TheSTL , I have made changes as per the review and also made a correction for the binary search file as it was creating child nodes for null value as well.