Better to follow a common stack API

The common API for a stack:

• push(item): add an item on the top of the stack and then return the item
• pop(): remove the top item from the stack and return it

My objection is against what your methods return.push returns a message,pop returns an(item, message) tuple.If you think about it, the message is useless.Users of this class would have to check the output of each call,and evaluate success or failure using string comparison.This is very weak.

Instead of using the return value to indicate success and failure,it would be better to follow the common API,and raise exceptions in case of failures. For example like this:

class EmptyStackError(Exception):    def __init__(self):        super().__init__("Stack is empty: cannot pop an empty stack")class StackFullError(Exception):    def __init__(self):        super().__init__("Stack is full")class stack():    # ...    def push(self, data):        if self.isFull():            raise StackFullError()        self.data.append(data)        return data    def pop(self):        if self.isEmpty():            raise EmptyStackError()        item = self.data[len(self.data) -1]        del self.data[len(self.data) -1]        return item

Python conventions

FollowPEP8, the official coding style guide of Python. For example:

• The convention for class names isPascalCase, sostack should beStack
• The convention for method names issnake_case, soisFull should beis_full

Hard coding a magic number for stack size has code smell.

Stack semantics derive from automata theory. As an abstraction, stacks do not have a fixed size [of 16 or anything else] and cannot be filled only emptied. An object oriented implementation should perhaps reflect this at the top of the inheritance hierarchy to avoid conflating implementation details with stack semantics.

When it is necessary to implement a stack of limited size, consider taking size as a parameter to the constructor not hard coded as a magic number. This will allow greater code reuse and more clearly separate implementation dependencies from the higher level of abstraction.

Finally, why not implement stack as alist?

Example code:

class Stack():    def __init__(self):        self.stack = []    def push(self, val):        self.stack.append(val)    def pop(self):        try:            return self.stack.pop()        except IndexError:            print "Sorry, cannot pop an empty stack"

At this point, we are free to implement specific business logic such as that for modeling an HP11C calculator (I've had mine since 1988).

class MyStack(Stack):    def __init__(self, size):        Stack.__init__(self)        self.size = size    def push(self, val):        if len(self.stack) < self.size:            Stack.push(self, val)        else:            print "Sorry, stack is full"    def peek(self):        temp = self.pop()        self.push(temp)        return temp    def is_empty(self):        return len(self.stack) == 0    def flush(self):        self.stack = []

A sound inheritance model allows forC style stack semantics wherepop simply deletes the top of the stack without returning a value andpeek is the way to read the top of the stack.

class MyCStyleStack(MyStack):    def pop(self):        MyStack.pop(self)

• \$\begingroup\$The code is for use in an RPN calculator project I am doing on an AVR. I wrote it first in python so I could wrap my head around what I would have to write in C++. Traditionally RPN calculators (or at lest the HP ones I have owned) only had a 4-16 index stack. Because of storage limitations I needed to have an arbitrary limit to the stack size.\$\endgroup\$

  Billylegota

  – Billylegota

  2015-02-28 16:34:27 +00:00

  CommentedFeb 28, 2015 at 16:34
• 1
  \$\begingroup\$@Billylegota To me, implementing code to understand the mechanics of a system is all the more reason to separate the data structure from the implementation details [see edited answer]. YMMV.\$\endgroup\$

  ben rudgers

  – ben rudgers

  2015-02-28 20:55:48 +00:00

  CommentedFeb 28, 2015 at 20:55

This method can be simplified:

def isEmpty(self):    if len(self.data) == 0:        return True    else:        return False

This is sufficient:

def isEmpty(self):    return len(self.data) == 0

The same applies toisFull(self).

The rest looks good to me, except you should be consistent about using using spaces around your operators according to thePEP 8 style:

output = self.data[len(self.data) -1]

• 1
  \$\begingroup\$maxSize=16 is already using thestandard spacing (though not the recommended capitalization).\$\endgroup\$

  200_success

  – 200_success

  2015-02-28 07:59:37 +00:00

  CommentedFeb 28, 2015 at 7:59
• 2
  \$\begingroup\$maxSize=16, being a keyword argument, follows PEP8 as it is (in terms of spacing, at least). Adding spaces there would violate PEP8. It's not that this is "inconsistent", it's that some rules depend on the context.\$\endgroup\$

  janos

  – janos

  2015-02-28 16:43:44 +00:00

  CommentedFeb 28, 2015 at 16:43
• \$\begingroup\$@janos Oh, that is interesting.\$\endgroup\$

  user34073

  – user34073

  2015-02-28 16:59:32 +00:00

  CommentedFeb 28, 2015 at 16:59

• python
• python-2.x
• stack