Stack Implementation in Python

Question

I have written a simple stack implementation and would like some feedback as to what I could improve in my code and coding practices.

# A simple stack in python with ints.
class stack():
 def __init__(self, maxSize=16):
 self.maxSize = maxSize
 self.data = []
 def isEmpty(self):
 if len(self.data) == 0:
 return True
 else:
 return False
 def isFull(self):
 if len(self.data) == self.maxSize:
 return True
 else:
 return False
 def push(self, data):
 if not self.isFull():
 self.data.append(data)
 return "OK"
 else:
 return "ERR_Stack_Full"
 def pop(self):
 if not self.isEmpty():
 output = self.data[len(self.data) -1]
 del self.data[len(self.data) -1]
 return output, "OK"
 else:
 return "ERR_Stack_Empty"
 def npop(self, n):
 output = []
 if len(self.data) >= n:
 for i in range(n):
 output.append(self.pop()[0]) 
 return output, "OK"

Based on the input given here is my modified code (Sorry about the entire thing being indented, the code sample button was being difficult):

 class EmptyStackError(Exception):
 def __init__(self):
 super().__init__("Stack is empty: cannot pop from an empty stack!")
 class FullStackError(Exception):
 def __init__(self):
 super().__init__("Stack is full: cannot push to a full stack!")
 # A simple stack in python with ints.
 class Stack():
 def __init__(self, max_size=16):
 self.max_size = max_size
 self.data = []
 def is_empty(self):
 if len(self.data) == 0:
 return True
 def is_full(self):
 if len(self.data) == self.max_size:
 return True
 def push(self, data):
 if not self.is_full():
 self.data.append(data)
 return data
 else:
 raise FullStackError()
 def pop(self):
 if not self.is_empty():
 output = self.data[len(self.data) -1]
 del self.data[len(self.data) -1]
 return output
 else:
 raise EmptyStackError() 

Answer 1

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

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

• The convention for class names is PascalCase, so stack should be Stack
• The convention for method names is snake_case, so isFull should be is_full

Answer 2

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 a list?

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 for C style stack semantics where pop simply deletes the top of the stack without returning a value and peek is the way to read the top of the stack.

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

• Billylegota: 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.
• ben rudgers: @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.

Answer 3

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 to isFull(self).

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

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

• 200_success: maxSize=16 is already using the standard spacing (though not the recommended capitalization).
• janos: 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.
• user34073: @janos Oh, that is interesting.