3
\$\begingroup\$

I have made a stack and queue implementation with OOP in Python (sorry for variable names) . It is made from scratch. I just watched a video how these two things work. I am still trying to improve in OOP so I would be thankful for every advice. Two things which bother me the most are using que and sta in the beginning and whether it is okay to just use already implemented functions in Python such as .pop() .

QUEUE

que = []

class Queue:
    def __init__(self,queue):
        self.queue = queue

    def push(self,item):
        print ("PUSH",item)

        if len(self.queue)!=0:
            self.queue.append(0)
            for i in range(len(self.queue)-1,0,-1): ## replacing elements 
                self.queue[i] = self.queue[i-1]
            self.queue[0] = item

        else:
            self.queue.append(item)

    def pop(self):
        print ("POP",self.queue[-1])

        if len(self.queue)!=0:
            self.queue.pop()
        else:
            raise ValueError('Cant pop from empty queue.')

    def __str__(self):
        return (str(self.queue))

## creating QUEUE  
que = Queue(que)
que.push(5)
print (que)
que.push(6)
print (que)
que.push(7)
print (que)
que.pop()
print (que)

STACK

class Stack:
    def __init__(self,stack):
        self.stack = stack

    def push(self,item):
        print ("PUSH",item)

        if len(self.stack)!=0:
            self.stack.append(0)
            for i in range(len(self.stack)-1,0,-1):
                self.stack[i] = self.stack[i-1]
            self.stack[0] = item

        else:
            self.stack.append(item)

    def pop(self):
        print ("POP",self.stack[0])

        if len(self.stack)!=0:
            self.stack.pop(0)
        else:
            raise ValueError('Cant pop from empty stack.')

    def __str__(self):
        return (str(self.stack))
## creating STACK
stac = Stack(sta)
stac.push(5)
print(stac)
stac.push(7)
print(stac)
stac.push(2)
print(stac)
stac.pop()
print(stac)
\$\endgroup\$
  • \$\begingroup\$ What is que? What is sta? As presented, the code doesn't run. \$\endgroup\$ – vnp Feb 13 '18 at 21:08
  • 1
    \$\begingroup\$ @vnp que is defined in the first statement. Assuming that sta is similarly defined, this code is reviewable. \$\endgroup\$ – 200_success Feb 13 '18 at 21:30
4
\$\begingroup\$

There are a few things to mention.

At first, it might be a good idea to initialize both Queue and Stack with empty lists by default:

def __init__(self, queue=None):
    self.queue = queue or []    # if queue is given, it overrides the default

Next, it doesn't make sense to write something like if len(self.queue)!=0 since it is equivalent to if self.queue (empty list is False when it's casted to bool). You can go even further and introduce the property called size using the built-in decorator:

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

Now it can be accessed by self.size without a function call.

\$\endgroup\$
  • 2
    \$\begingroup\$ or you can implement __len__ instead of size… now your container behave like any other as regard to the len() builtin. \$\endgroup\$ – 409_Conflict Feb 14 '18 at 9:22
3
\$\begingroup\$

Adopting an existing list is generally a bad idea. Code outside of the Queue and Stack classes could still hold a reference to the list and manipulate it behind your back. The simplest remedy is to always construct the object as an empty data structure.

In your Stack, adding and removing items at the front of a list (at index 0) gives the worst performance, since every existing element needs to be shifted over. This work is clearly visible in your .push(), but less obviously, self.stack.pop(0) also does that kind of work. For efficiency, you should append and truncate items at the end of the list instead.

\$\endgroup\$

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.