-
Notifications
You must be signed in to change notification settings - Fork 0
Dequeue #6
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
base: master
Are you sure you want to change the base?
Dequeue #6
Conversation
… into dequeue merge with jordan's code
… into dequeue merge with jordan's tests
src/deque.py
Outdated
| return self.dll.shift() | ||
| self.head_node = self.dll.head_node | ||
| self.tail_node = self.dll.tail_node | ||
| self.length = self.dll.length |
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.
I suspect the deque pop / dll shift method might be not be working as expected...
In [2]: dq = Deque([1, 2, 3, 4])
In [3]: dq.pop()
Out[3]: 1
In [4]: dq.pop()
---------------------------------------------------------------------------
ValueError: Linked list is already emptywhile this works:
In [2]: dq = Deque([1, 2, 3, 4])
In [3]: dq.popleft()
Out[3]: 4
In [4]: dq.popleft()
Out[4]: 3
In [5]: dq.popleft()
Out[5]: 2
In [6]: dq.popleft()
Out[6]: 1
src/deque.py
Outdated
| return self.dll.shift() | ||
| self.head_node = self.dll.head_node | ||
| self.tail_node = self.dll.tail_node | ||
| self.length = self.dll.length |
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.
also:
In [7]: dq = Deque([1, 2])
In [8]: dq.popleft()
Out[8]: 2
In [9]: dq.popleft()
Out[9]: 1
In [10]: dq.pop()
Out[10]: 1
src/deque.py
Outdated
|
|
||
| def peek(self): | ||
| """Display but don't remove the contents of tail node.""" | ||
| return self.dll.tail_node.contents |
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.
Peek should return none from an empty deque (see assignment page)
In [2]: dq = Deque()
In [3]: dq.peek()
---------------------------------------------------------------------------
AttributeError: 'NoneType' object has no attribute 'contents'
src/deque.py
Outdated
|
|
||
| def peekleft(self): | ||
| """Display but don't remove the contents of head node.""" | ||
| return self.dll.head_node.contents |
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.
Peekleft should also return none from an empty deque (see assignment page)
In [4]: dq = Deque()
In [5]: dq.peekleft()
---------------------------------------------------------------------------
AttributeError: 'NoneType' object has no attribute 'contents'
src/test_deque.py
Outdated
| one_deque = Deque(["one"]) | ||
| empty_deque = Deque() | ||
| new_deque = Deque([1, 2, 3, 4, 5]) | ||
| return one_deque, empty_deque, new_deque |
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.
Split this into three fixtures. Look into using parametrize to run each deque + expected output though a set of test functions if they should all behave the same (output val vs output error). Will make testing more clear / focused.
src/test_deque.py
Outdated
|
|
||
| def test_deque_size_on_one(sample_deque): | ||
| """Test for size on empty deque.""" | ||
| assert sample_deque[0].size() == 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.
Check Docstring (which deque is this test for)
src/test_deque.py
Outdated
|
|
||
| def test_deque_peekleft_on_one(sample_deque): | ||
| """Test for peekleft on empty deque.""" | ||
| assert sample_deque[0].peekleft() == 'one' |
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.
Check docstring (which deque is this for)
src/test_deque.py
Outdated
|
|
||
| def test_deque_peek_on_one(sample_deque): | ||
| """Test for peek on empty deque.""" | ||
| assert sample_deque[0].peek() == 'one' |
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.
Check docstring (which deque is this for)
src/test_deque.py
Outdated
|
|
||
| def test_deque_pop_on_one(sample_deque): | ||
| """Test for pop on empty deque.""" | ||
| assert sample_deque[0].pop() == 'one' |
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.
Check docstring (which deque is this for)
src/test_deque.py
Outdated
| def test_deque_append_on_one(sample_deque): | ||
| """Test for append on empty deque.""" | ||
| sample_deque[0].append("hey") | ||
| assert sample_deque[0].tail_node.contents == "hey" |
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.
Check docstring (which deque is this for)
src/test_deque.py
Outdated
| def test_deque_appendleft_on_one(sample_deque): | ||
| """Test for appendleft on empty deque.""" | ||
| sample_deque[0].appendleft("hey") | ||
| assert sample_deque[0].head_node.contents == "hey" |
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.
Check docstring (which deque is this for)
|
|
||
| def append(self, contents): | ||
| """Add value to the end of the deque.""" | ||
| self.dll.append(contents) |
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.
Something isn't hooked up right under the hood...
In [3]: dq = Deque()
In [4]: dq.append(1)
In [5]: dq.append(2)
In [6]: dq.dll.tail_node.previous_node
Out[6]: <__main__.Node at 0x7fdf70aea630>
In [7]: dq.dll.head_node.previous_node
In [8]: dq.dll.head_node.next_node
In [9]: dq.dll.tail_node.next_nodeThere 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.
Either head or tail should have a next node.
| def test_deque_empty_init(empty_deque): | ||
| """Test for empty deque init.""" | ||
| assert empty_deque.dll.head_node is None | ||
| assert empty_deque.dll.tail_node is None |
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.
Split this into two tests. One that confirms head_node and one that confirms tail_node.
| def test_deque_one_init(single_deque): | ||
| """Test for one item deque init.""" | ||
| assert single_deque.dll.head_node.contents == "one" | ||
| assert single_deque.dll.tail_node.contents == "one" |
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.
Split this into two tests. One that confirms head_node and one that confirms tail_node.
| def test_deque_init(sample_deque): | ||
| """Test for new deque init.""" | ||
| assert sample_deque.dll.head_node.contents == 5 | ||
| assert sample_deque.dll.tail_node.contents == 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.
Split this into two tests. One that confirms head_node and one that confirms tail_node.
src/test_deque.py
Outdated
|
|
||
| def test_deque_size_increase(sample_deque): | ||
| """Test that size increases after appending.""" | ||
| testing_deque = sample_deque |
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.
Why are you binding sample_deque to testing_deque. The purpose of fixtures is to have an object ready to use for a single test. Remove this line and use sample_deque like the rest of the tests you run.
src/test_deque.py
Outdated
|
|
||
| def test_deque_size_decrease(sample_deque): | ||
| """Test that size decreases after appending.""" | ||
| testing_deque = sample_deque |
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.
See above.
No description provided.