HELP! Stack implementation


#1

Im working on a Stack that uses deques, but I am not sure if I am implementing it right specifically my pop method. I could I have some feedback?

class Stack:

  def __init__(self):
      self._dq = get_deque()


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

  def __len__(self):
      return len(self._dq)

  def push(self, val):
      if self._dq.__len__() != 0:
          self._dq.push_front(val)
      else:
          self._dq.push_back(val)

  def pop(self):
      if self._dq.__len__() <= 0:
          return
      else:
          self._dq.pop_front()


  def peek(self):
      if self._dq.__len__() <= 0:
          return
      else:
          return self._dq.peek_front()

#2

Hello :slight_smile:

When you ask for code review or for testing you should provide executable snippet of your code. From your code I assume that get_deque() returns deque instance, but by method names I can tell that this is your own implementation. So it would be helpfull for us to see it.


This method looks a bit off:

def push(self, val):
  if self._dq.__len__() != 0:
    self._dq.push_front(val)
  else:
    self._dq.push_back(val)

If deque size equals 0 both methods (push_front and push_back) should have exactly the same result.

So you can simplify your push:

def push(self, val):
  self._dq.push_front(val)

But it depends on your implementation of deque, maybe it’s faulty.


if self._dq.__len__() <= 0:

len should not return negative values, so it would be better to change the condition to == 0. It makes code more readable.

If I want to pop empty stack I should get an exception. In your implementation I don’t get any feedback. That is wrong. If operation I executed failed I should know about that and I should be able to execute different code that will handle the exception. So instead of empty return statement you should raise an exception.


In Python, according to some conventions and style guides, if you implement a container there should be a way to handle list as input. In other words, push should operate also as extend. I don’t know if this is implemented in your code because I don’t see deque implementation.