Python3 Stack implementation with List built-in

Question

So, inspired by this other code review, I wanted to see if I could implement a very basic stack without the linked-list approach taken by the OP in their implementation.

To that end, I came up with the following Stack class. I wrote it for Python 3, but it actually works as-is in Python 2 as well:

class Stack:
    _stack = []

    def __init__(self):
        self._stack = []

    @property
    def size(self):
        return len(self._stack)

    @property
    def count(self):
        return self.size

    def __sizeof__(self):
        return self.size

    def pop(self):
        if self.size == 0:
            return "Cannot pop from an empty stack."

        item = self._stack[self.size - 1]
        self._stack.remove(item)
        return item

    def peek(self):
        if self.size == 0:
            return "Cannot peek into an empty stack."

        return self._stack[self.size - 1]

    def push(self, item):
        self._stack.append(item)

    def __iter__(self):
        return iter(self._stack)

I'm probably missing some key parts of what one would want from a Stack, but any improvement is welcome. Literally any improvement.

Note there were a couple things I did intentionally:

1. I intentionally initialize the _stack attribute as an empty list both in the class itself and in __init__. For some reason PyCharm complains if the attribute isn't already part of the class when being worked with in __init__, so this is more or less to get PyCharm to stop yelling at me.
2. I intentionally have __sizeof__ declared as referring to the size property function. This lets len(StackObject) work if someone doesn't want to do stack.size.

With these in mind, feel free to tell me what you think, or point out any mistakes I've made. (I'm not an expert in data structures, so I may have gotten some points messed up a little).

Answer 1

By far the biggest problem this code has is error handling. When popping or peeking from an empty stack this returns a string. this is really broken. For an example of why, suppose I add the string "Cannot peek into an empty stack." to the stack and then try to see what the top element is. I won't know if the answer I got was because the stack was empty, or because that string was in the stack. The solution to this is to raise an IndexError when the stack is empty. I also think that you shouldn't define either size or count, as the pythonic way of getting somethings length is to call len(x). You might also be interested to know that python lists have a pop function already, so you could just use that for your pop

Answer 2

The implementation looks pretty clean and simple to me, but here's what caught my attention:

• I think you should sacrifice PyCharm complaining about accessing attributes in __init__ (which is very weird) for readability here. It's just one line of code, but it's pretty confusing since _stack is obviously bound to an instance.

• obj.__sizeof__ != obj.__len__, which is really what you're supposed to override for len(obj) and sys.getsizeof(obj) to work as expected.

  Try this:

  >>> class Foo:
  ...     def __init__(self, x):
  ...             self.x = x
  ...     def __sizeof__(self):
  ...             return 100
  ...     def __len__(self):
  ...             return len(self.x)
  ...
  >>>
  >>> class Bar:
  ...     def __init__(self, x):
  ...             self.x = x
  ...     def __sizeof__(self):
  ...             return 200
  ...     def __len__(self):
  ...             return len(self.x)
  ...
  >>> f = Foo([1,2,3])
  >>> b = Bar([1,2,3,4])
  >>> assert len(f) == 3
  >>> assert len(b) == 4
  

  So far, so good. Calling len on instances of Foo and Bar works just fine. But what happens when we use sys.getsizeof?

  >>> import sys
  >>> sys.getsizeof(f)
  124
  >>> sys.getsizeof(b)
  224
  

  (The added 24 bytes are garbage collector overhead).

• Functions shouldn't return an error message. I would expect pop and seek to return None, or throw an exception. Returning strings is dangerous, especially in this case, where they could be a valid return value, even if nothing went wrong.

Answer 3

I intentionally have __sizeof__ declared as referring to the size property function. This lets len(StackObject) work if someone doesn't want to do stack.size.

From The Zen of Python:

There should be one-- and preferably only one --obvious way to do it.

Python likes there to be One True Way to do things, because it keeps usage consistent everywhere.

In this case, I would do away with the size and count properties entirely, and just define a __len__ (not __sizeof__, like other answers have mentioned). Using __len__ (or rather, the len() function) is the standard way to find the size of a container in Python, so you should provide this method and nothing else.

Instead of:

...

@property
def size(self):
    return len(self._stack)

@property
def count(self):
    return self.size

def __sizeof__(self):
    return self.size

...

Do this:

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