HiveBrain v1.2.0
Get Started
← Back to all entries
patternpythonMinor

Stack Implementation in Python

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
implementationpythonstack

Problem

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():

Solution

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

Code Snippets

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

Context

StackExchange Code Review Q#82802, answer score: 5

Revisions (0)

No revisions yet.