3
\$\begingroup\$

Previous code is available at Python3 Stack implementation with List built-in, this is a second-round review after I made alterations.

Once again, let's review my Stack implementation, with built-in lists, and with review notes from the prior review implemented.

Any changes and suggestions are welcome!



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

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

    def pop(self):
        if len(self) == 0:
            raise IndexError("You cannot pop from an empty stack.")

        return self._stack.pop()

    def peek(self):
        if len(self) == 0:
            raise IndexError("You cannot peek into an empty stack.")

        return self._stack[len(self) - 1]

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

    def __iter__(self):
        return iter(self._stack)
\$\endgroup\$
  • \$\begingroup\$ Shouldn't __iter__ iterate the list in reverse order? \$\endgroup\$ – tobias_k Dec 9 '17 at 22:29
6
\$\begingroup\$
  • If you don't care if the error is textually correct, then you can remove the if in pop and peek.
  • You don't need to find the length of the stack to find the last element. You can just use self._stack[-1].

Creating:

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

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

    def pop(self):
        return self._stack.pop()

    def peek(self):
        return self._stack[-1]

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

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

From this, it begs the question why not inherit from list? Which would make the code:

class Stack(list):
    def peek(self):
        return self[-1]

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

If you're likely to switch to another form of Stack, then you may want to keep with this. However, for the most part I'd recommend not using this class. As the only benefit it has over list is that it's likely to work with more Stack interfaces.

\$\endgroup\$
  • \$\begingroup\$ Except that you can't really implement a stack with just a pure list. This was more an exercise than anything else, in 99.999% of all cases you hould just use a list :p \$\endgroup\$ – Thomas Ward Dec 9 '17 at 19:46
  • 2
    \$\begingroup\$ @ThomasWard If you wanted to reinvent-the-wheel, then you should actually implement a stack, rather than interface it. \$\endgroup\$ – Peilonrayz Dec 9 '17 at 19:48
  • \$\begingroup\$ True statement. :) (That said, I prefer Python lists, dicts, sets, tuples, etc. depending on my need. Trace back far enough and you'll see that someone wrote a linked-list stack implementation which was blergh, the comments on that were along the lines of "Just use a list" and then we now get to the "Don't reinvent the wheel" thing. \$\endgroup\$ – Thomas Ward Dec 9 '17 at 19:50
  • 1
    \$\begingroup\$ @ThomasWard If you "wrote a linked-list stack implementation" and tagged it as reinventing-the-wheel then you wouldn't get 'just use list'. \$\endgroup\$ – Peilonrayz Dec 9 '17 at 19:54
  • \$\begingroup\$ Maybe I'm just out of date on this whole "Python" thing, but who on earth would suggest to replace a very specific and clear error message with a general index-out-of-range one? Why not use the class? It expresses the intent that I want a stack and not a list. Inheriting from list gives you more power than the stack implementation in the OP: don't use as a list if you want a stack. None of these suggestions strike me as having any value, excepting the cleaner last-element lookup. \$\endgroup\$ – VisualMelon Dec 10 '17 at 14:28

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.