Skip to content
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

Implement exercise binary-search-tree #773

Merged
merged 31 commits into from
Nov 13, 2017
Merged
Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
cae11fe
Implement exercise binart-search-tree
rodrigocam Oct 8, 2017
d8256db
Add TreeNode class and binary search tree add method
rodrigocam Oct 8, 2017
5b65169
Add preorder, postorder and inorder traversal
rodrigocam Oct 8, 2017
06b303f
Add search method
rodrigocam Oct 8, 2017
b39e8d8
Add some tests
rodrigocam Oct 8, 2017
a5350dc
Add more testes, examle file and a readme
rodrigocam Oct 8, 2017
5604b08
Fix flake8 diretives
rodrigocam Oct 8, 2017
1013ec8
Add a new entry to config.json
rodrigocam Oct 8, 2017
5883681
Change new key place
rodrigocam Oct 8, 2017
ab2fa3e
Change search paramater name and add early if condition to exit if th…
rodrigocam Oct 10, 2017
1425ffc
Change exercise position in config.json
rodrigocam Oct 27, 2017
cb4e3bf
Merge branch 'master' into master
cmccandless Oct 27, 2017
fed194a
Merge branch 'master' into master
cmccandless Oct 27, 2017
9990ec2
Merge branch 'master' into master
cmccandless Oct 27, 2017
7192861
fix config.json syntax error
cmccandless Oct 27, 2017
3fe384e
Add blank line
rodrigocam Nov 3, 2017
62c5933
Add title at readme
rodrigocam Nov 3, 2017
49f6701
Add necessary info at readme
rodrigocam Nov 3, 2017
10627f2
Add skeletons for add and search methods
rodrigocam Nov 3, 2017
f4d558f
Change assert used by tests
rodrigocam Nov 3, 2017
5663d14
Add list method
rodrigocam Nov 3, 2017
56f971b
Change tests to include the new list method
rodrigocam Nov 3, 2017
fa1de56
Merge branch 'master' into master
cmccandless Nov 3, 2017
805557a
Fix flake8 guidelines
rodrigocam Nov 9, 2017
3b916a5
Change search and add method
rodrigocam Nov 9, 2017
1332fdc
Merge branch 'master' of https://github.com/rodrigocam/python
rodrigocam Nov 9, 2017
cd94b1f
Change variable name in search method and delete .cache directory
rodrigocam Nov 13, 2017
18c399f
Remove spaces on blank line
rodrigocam Nov 13, 2017
a01cb5a
Change parameter of search method
rodrigocam Nov 13, 2017
c054b29
Remove print methods
rodrigocam Nov 13, 2017
d43a2ba
Merge branch 'master' into master
Nov 13, 2017
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 15 additions & 2 deletions config.json
Original file line number Diff line number Diff line change
Expand Up @@ -1112,6 +1112,20 @@
"exception_handling"
]
},
{
"uuid": "6f196341-0ffc-9780-a7ca-1f817508247161cbcd9",
"slug": "binary-search-tree",
"core": false,
"unlocked_by": null,
"difficulty": 4,
"topics":[
"recursion",
"classes",
"trees",
"searching",
"object_oriented_programming"
]
},
{
"uuid": "e7351e8e-d3ff-4621-b818-cd55cf05bffd",
"slug": "accumulate",
Expand Down Expand Up @@ -1166,9 +1180,8 @@
"uuid": "6b7f7b1f-c388-4f4c-b924-84535cc5e1a0",
"slug": "hexadecimal",
"deprecated": true
}
},
Copy link
Contributor

Choose a reason for hiding this comment

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

It would probably be best to move the config.json entry for this exercise before all deprecated exercises.

],
"foregone": [

Copy link
Contributor

Choose a reason for hiding this comment

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

Can you add this blank line back in?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

Copy link
Contributor

Choose a reason for hiding this comment

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

I looks like you've got some unnecessary white-space here now (see below) - can you please remove it?

]
}
52 changes: 52 additions & 0 deletions exercises/binary-search-tree/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
Insert and search for numbers in a binary tree.
Copy link
Contributor

Choose a reason for hiding this comment

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

This needs the title of the exercise as a heading: # Binary Search Tree

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done


When we need to represent sorted data, an array does not make a good
data structure.

Say we have the array `[1, 3, 4, 5]`, and we add 2 to it so it becomes
`[1, 3, 4, 5, 2]` now we must sort the entire array again! We can
improve on this by realizing that we only need to make space for the new
item `[1, nil, 3, 4, 5]`, and then adding the item in the space we
added. But this still requires us to shift many elements down by one.

Binary Search Trees, however, can operate on sorted data much more
efficiently.

A binary search tree consists of a series of connected nodes. Each node
contains a piece of data (e.g. the number 3), a variable named `left`,
and a variable named `right`. The `left` and `right` variables point at
`nil`, or other nodes. Since these other nodes in turn have other nodes
beneath them, we say that the left and right variables are pointing at
subtrees. All data in the left subtree is less than or equal to the
current node's data, and all data in the right subtree is greater than
the current node's data.

For example, if we had a node containing the data 4, and we added the
data 2, our tree would look like this:

4
/
2

If we then added 6, it would look like this:

4
/ \
2 6

If we then added 3, it would look like this

4
/ \
2 6
\
3

And if we then added 1, 5, and 7, it would look like this

4
/ \
/ \
2 6
/ \ / \
1 3 5 7
Copy link
Contributor

Choose a reason for hiding this comment

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

There should also be some other stuff that goes here. Check another exercise and copy it over, or regenerate this README with the configlet.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

8 changes: 8 additions & 0 deletions exercises/binary-search-tree/binary_search_tree.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
class TreeNode(object):
def __init__(self, value):
pass
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you add self.value = value here? I feel like if we assume that something exists in the tests, then we should make it clear in the solution template.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done



class BinarySearchTree(object):
def __init__(self):
pass
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you also include skeletons for the add and search methods here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

35 changes: 35 additions & 0 deletions exercises/binary-search-tree/binary_search_tree_test.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
import unittest

from binary_search_tree import BinarySearchTree


class BinarySearchTreeTests(unittest.TestCase):
bst = BinarySearchTree()

def test_add_integer_number1(self):
self.assertTrue(self.bst.add(7))
Copy link
Contributor

Choose a reason for hiding this comment

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

We use assertIs(actual, True) and assertIs(actual, False) rather than assertTrue(actual) or assertFalse(actual) (#419).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done


def test_add_integer_number2(self):
self.assertTrue(self.bst.add(1))

def test_add_integer_number3(self):
self.assertTrue(self.bst.add(6))

def test_add_integer_number4(self):
self.assertTrue(self.bst.add(5))

def test_add_float_number(self):
self.assertTrue(self.bst.add(7.5))

def test_search_valid_number1(self):
self.assertEqual(self.bst.search(7).value, 7)

def test_search_valid_number2(self):
self.assertEqual(self.bst.search(7.5).value, 7.5)

def test_search_valid_number3(self):
self.assertEqual(self.bst.search(1).value, 1)


if __name__ == '__main__':
unittest.main()
69 changes: 69 additions & 0 deletions exercises/binary-search-tree/example.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
class TreeNode(object):
def __init__(self, value):
self.value = value
self.left_node = None
self.right_node = None

def __str__(self):
return str(self.value)


class BinarySearchTree(object):
def __init__(self):
self.root = None

def add(self, value):
if(self.root is None):
self.root = TreeNode(value)
inserted = True
return inserted
else:
inserted = False
cur_node = self.root

while not inserted:
if(value < cur_node.value):
if(cur_node.left_node):
cur_node = cur_node.left_node
else:
cur_node.left_node = TreeNode(value)
inserted = True
return inserted
elif(value > cur_node.value):
if(cur_node.right_node):
cur_node = cur_node.right_node
else:
cur_node.right_node = TreeNode(value)
inserted = True
return inserted
Copy link
Contributor

@N-Parsons N-Parsons Nov 2, 2017

Choose a reason for hiding this comment

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

Looking at this again, I just noticed that this method is returning True/False to indicate success/failure. Under what condition is failure expected to occur? From what I can see, this code will never return False, and would actually just enter an infinite loop if value == cur_node.value. I don't think that we should use booleans in this way - exceptions are a better method for handling errors in Python, but I don't think that any exceptions should be expected to occur here.

I think it would be worth adding a BinarySearchTree.list method to this exercise that returns a sorted list of values in the tree. A collections.deque would be most efficient for this, but I don't think that the tests should enforce its use, so the tests should probably convert the actual output to a list (eg. assertEqual(list(self.bst.list()), [1, 2, 3])).

Doing this would allow us to check that values are placed in the right order - each test for adding a number should contain a second assert for the current list of stored values. We should also test duplicate values, because the README specifies a way for these to be handled.

From the README:

All data in the left subtree is less than or equal to the current node's data, and all data in the right subtree is greater than the current node's data.

Copy link
Contributor

Choose a reason for hiding this comment

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

@N-Parsons Returning True from an add or insert method is a common practice for container classes. I agree that it doesn't make the most sense, but that is likely why @rodrigocam did this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@cmccandless is right, I thought this could be good to give more safety. I made the list method that @N-Parsons asked and change the tests to use this new method. I fix the infinite loop condition that you point but I keep the bool return, if it was a problem I can remove (:

Copy link
Contributor

@N-Parsons N-Parsons Nov 5, 2017

Choose a reason for hiding this comment

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

Given that there's no way that False can be returned, and there is no test for this method returning False, I think it's unnecessary and likely to cause some confusion/frustration. I think it's fine for this example code to return True/False, but I don't think that it should be tested for.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done! I take out the return of the method.


def search(self, search_number):
cur_node = self.root

Choose a reason for hiding this comment

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

Also add a check for if(self.root is None): return false which does an early exit if the tree hasn't been constructed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, checked!

found = False
while not found:
if(cur_node is None):
return False
elif(search_number < cur_node.value):
cur_node = cur_node.left_node
elif(search_number > cur_node.value):
cur_node = cur_node.right_node
elif(search_number == cur_node.value):
return cur_node

def print_inorder(self, node):
Copy link
Contributor

Choose a reason for hiding this comment

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

We should probably remove all print* methods, as they are not used by the tests.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done!

if(node is not None):
self.print_inorder(node.left_node)
print(node.value)
self.print_inorder(node.right_node)

def print_preorder(self, node):
if(node is not None):
print(node.value)
self.print_preorder(node.left_node)
self.print_preorder(node.right_node)

def print_postorder(self, node):
if(node is not None):
self.print_postorder(node.right_node)
self.print_postorder(node.left_node)
print(node.value)