Add priority heap implementation using lists.

[SaadBenn]

I know this is not an efficient way to do it but our repo was missing a priority queue implementation and I thought I might just just give it a try.

[coveralls]

Pull Request Test Coverage Report for Build 220

• 0 of 0 (NaN%) changed or added relevant lines in 0 files are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage remained the same at 91.076%

---

Totals
Change from base Build 217:	0.0%
Covered Lines:	2674
Relevant Lines:	2936

---

💛 - Coveralls

[danghai]

I already did it: https://github.com/keon/algorithms/blob/master/heap/binary_heap.py

[SaadBenn]

That implementation is with a heap which is a much more efficient way of implementing a priority queue however I think It's always a good idea to have different ways of implementing the same data structure. It just opens up your mind to so many different solutions. Just my two cents. @danghai

[danghai]

I thought it is the same algorithm. Priority queue require 2 popular method percolate down and percolate up. Your insert method looks like percDown. I think min/max heap is the same name priority queue

[SaadBenn]

Yep, mine is bubble down. I am adding context to this comment cuz danghai took it literally. When I said bubble down, what I meant was that I add items based on the lowest priority and for that I am doing comparison to get to appropriate spot (similar to how you'd do a find min in array)

[danghai]

So it looks the same file that I give you. If you can use linkedlist ( different data structure). It could be a different approach because the file I gave you use a array or a list

[SaadBenn]

No, your implementation is based off of heap (binary structure). This one is on lists which is not the best in terms of running time but everything has its tradeoffs.

[danghai]

Could you point out the difference my way and your way? The data structure is the same ( array , and you use list). The algorithm for perceDown is same too.

[SaadBenn]

The difference is what you use for implementation. Yours is heap which is a tree structure, this one is list even though you're using lists but the implementation is of a tree structure. Here is a much more detailed explanation of implementation of PQ using heaps.

[danghai]

It is the same! Binary tree structure explain the list/array store these number. Look your file, does it has binary tree structure?

[SaadBenn]

The implementation that I have doesn't have binary tree structure. It is just a simple but terrible implementation of PQ using lists. The idea is to have as many ways of implementing the same data structure. If you click on the link that I gave you, you should be able to see the difference.

[danghai]

Could you point out my binary tree structure in my code? :)

[SaadBenn]

First of all, you have perc_down and perc_up methods and every time you add a new item, you have to swap items with parent node by either bubbling up or down. In my case, I am not doing that. Mine is just doing a simple comparison with no bubbling up or down. I said mine is doing bubbling down in my first comment but I didn't explain that It's just a comparison. Bubbling up or bubbling down require comparison with the parent nodes and then pushing the value to the right place. Here is another link which might help you understand the difference.

[danghai]

So you give me this link: http://pages.cs.wisc.edu/~vernon/cs367/notes/11.PRIORITY-Q.html. And you implement base on it. Do you see and read these example for bubble Up/Down (percUP/Down)?

[SaadBenn]

That link contains the implementation of PQ using heaps. That was for your understanding. My implementation is not based on that as I am not doing it that way.

[danghai]

Furthermore, your method delete is wrong because it misses percDown to reconstruct the tree in array/list. Hopefully you know what I mean. I see it has the same data structure + the same algorithm. (It is just difference that I split them into small functions for clear). @goswami-rahul Could you review it?

[SaadBenn]

Bubble down is used when you're implementing your PQ using a tree structure.
A priority queue can have any implementation, like a list that you search linearly when you pop. All it means is that when you pop you get the value with either the minimum or the maximum depending.

A classic heap as it is typically referred to is usually a min heap. An implementation that has good time complexity (O(log n) on push and pop) and no memory overhead. So my implementation is based on lists/array.

[danghai]

You mean your priority queue always stores increasing order (MiN priority queue)

[SaadBenn]

Correct.

[goswami-rahul]

@danghai I think you got a little bit messed up between priority queues and max/min heaps :) Anyone can get confused here because priority queues almost everytime get implemented using heaps (for efficiency). However, the priority queue is just a data structure that stores a group of items and we can extract the min/max item based on the priority.
Your code achieves this by using the above binary heap data structure. (in O(log n) time),
while @SaadBenn did it here with just an array in which we can insert in O(n) keeping the array sorted, and get min/max in O(1) by returning the first/last element of the sorted array :)

[goswami-rahul]

So I guess we can merge now?

[SaadBenn]

Excellent!! Merging it now.

[danghai]

@goswami-rahul @SaadBenn I am not sure why you merge it, while he does not provide a unittest. Have you look review the code? For example: insert why return True in else? What does it mean self.data? He use data for what ? what is the test case? How do you know these method work?

[SaadBenn]

A node usually contains some data. If you look at the PriorityQueueNode class, you'll see that I am storing data and priority of the node in my list. The insert method takes a node of type PriorityQueueNode type (everything is an object in Python but just for clarity I am mentioning it here). As far as unittest is concerned, I'll try to add it as soon as possible (I was told that I don't need to do unittests but I'll add it anyway). Returning True is just good programming practice to know that something was successful. It is in the book called Clean coder. I'd highly recommend you to read it. :)

[danghai]

@SaadBenn I read these book for a long time ago. Return true only for else. What about return value for this if condition: if self.size() == 0: , return what?

[SaadBenn]

@goswami-rahul Can you please do another review? I'll make the adjustments. I'm sure you'll do an objective review. :)

[SaadBenn]

@danghai Let's leave the review to goswami.

[goswami-rahul]

@SaadBenn @danghai
Sorry for late code review. There's minor issue, where the insert returns True for some cases, and None for some other case. Since it is just a small change, I will open another PR to rectify this :)

[goswami-rahul]

@SaadBenn This entire insert method method can be rewritten as:

def insert(self, node):
    for index, current  in enumerate(self.priority_queue_list):
        if current.priority > node.priority:
            self.priority_queue_list.insert(index, node)
            return
    # when traversed complete queue
    self.priority_queue_list.append(node)

I think its better and more clear this way.

[goswami-rahul]

And yeah, there is no point of returning True here, because it would return True every time, and there is no case for False. So, its better to return None.
Returning True and False is more logical if there was, for example, a maxsize restriction in the queue. Then we would have returned True for every successful insertion, while returned False if the queue was already full :)

[goswami-rahul]

we can use deque for O(1) popleft.