Skip to content

Dequeue#6

Open
JSchatzman wants to merge 23 commits intomasterfrom
dequeue
Open

Dequeue#6
JSchatzman wants to merge 23 commits intomasterfrom
dequeue

Conversation

@JSchatzman
Copy link
Owner

No description provided.

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

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 empty

while 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

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

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

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'

one_deque = Deque(["one"])
empty_deque = Deque()
new_deque = Deque([1, 2, 3, 4, 5])
return one_deque, empty_deque, new_deque
Copy link

@bgarnaat bgarnaat Dec 23, 2016

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.


def test_deque_size_on_one(sample_deque):
"""Test for size on empty deque."""
assert sample_deque[0].size() == 1

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)


def test_deque_peekleft_on_one(sample_deque):
"""Test for peekleft on empty deque."""
assert sample_deque[0].peekleft() == 'one'

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 test_deque_peek_on_one(sample_deque):
"""Test for peek on empty deque."""
assert sample_deque[0].peek() == 'one'

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 test_deque_pop_on_one(sample_deque):
"""Test for pop on empty deque."""
assert sample_deque[0].pop() == 'one'

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 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"

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 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"

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)
Copy link

@bgarnaat bgarnaat Jan 2, 2017

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_node

Copy link

@bgarnaat bgarnaat Jan 2, 2017

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
Copy link

@bgarnaat bgarnaat Jan 2, 2017

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"
Copy link

@bgarnaat bgarnaat Jan 2, 2017

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
Copy link

@bgarnaat bgarnaat Jan 2, 2017

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_size_increase(sample_deque):
"""Test that size increases after appending."""
testing_deque = sample_deque
Copy link

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.


def test_deque_size_decrease(sample_deque):
"""Test that size decreases after appending."""
testing_deque = sample_deque
Copy link

Choose a reason for hiding this comment

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

See above.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants