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

D-ary max heap implementation

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

Problem

I implemented a D-ary max heap backed by a vector for resizing. I would like to know any possible improvements in performance, design, and in the code in general.

#pragma once
#include 
#include 
#include "Comparators.h"

template
class DHeap
{

public:
    DHeap();
    DHeap(unsigned);
    ~DHeap();

    T extractMax();
    void insert(const T&);
    T increaseKey(unsigned, const  T&);
    bool empty() const{ return vec.size() == 0; }

private:
    std::vector vec;
    int D;

    void maxheapify(unsigned);
    void swim(int);
    unsigned max(unsigned, unsigned) const;
    unsigned firstChild(unsigned) const;
    int parent(int) const;
    void ensureCapacity();
    int cmp(const T &, const T&) const;
    int cmpI(unsigned, unsigned) const;

    friend std::ostream & operator& p)
    {
        for (const T& d : p.vec)
            os 
DHeap::DHeap() : DHeap(4){}

template 
DHeap::DHeap(unsigned d) :D(d){}

template 
DHeap::~DHeap(){}

template 
T DHeap::extractMax()
{
    if (vec.size() 
void DHeap::insert(const T &element)
{
    ensureCapacity();
    vec.push_back(element);
    swim(vec.size()-1);
}

template 
T DHeap::increaseKey(unsigned index, const T &element)
{
    if (cmp(element, vec[index]) 
void DHeap::maxheapify(unsigned index)
{
    unsigned first = 0;
    while ((first = firstChild(index)) 
unsigned DHeap::max(unsigned a, unsigned b) const
{
    return cmpI(a, b) 
int  DHeap::cmp(const T & a, const T& b) const
{
    return Comparators::compare(a, b);
}
template
int DHeap::cmpI(unsigned i, unsigned j) const
{
    return cmp(vec[i], vec[j]);
}

template 
void DHeap::swim(int index)
{
    int p = 0;
    while (index > 0 && cmpI(p = parent(index), index) 
unsigned DHeap::firstChild(unsigned index) const
{
    return D* index + 1;
}
template 
int DHeap::parent(int index) const
{
    return (index - 1) / D;

}
template 
void DHeap::ensureCapacity()
{
    if (vec.size() == vec.capacity())
        vec.reserve(3 * vec.size() / 2 + 1);
}


This are some

Solution

Let's start with the signature:

template
class DHeap;


This is unsufficient. You provide some default comparators, which would be fine if my type happened to have operator largest at every iteration. And then I found your implementation. Let's not do it this way, and use the compare directly:

largest = compare(vec[largest], vec[first]) ? first : largest;


That is easy to understand. And let's actually factor out that loop that I wrote up there. So full rewrite of that function:

template 
void DHeap::maxheapify(unsigned index)
{
    while ((unsigned first = firstChild(index)) 
unsigned DHeap::findLargest(unsigned from, unsigned to)
{
    unsigned largest = from++;
    for (; from < to; ++from ) {
        largest = compare(vec[largest], vec[from]) ? from : largest;
    }
    return largest;
}


I find that much easier to parse. Note also the using std::swap. That let's ADL find swap() if necessary.

Code Snippets

template<typename T>
class DHeap;
template <typename T, typename Compare = std::less<T>>
class DHeap;
T const& maxElement() const; // or top()
void pop();
T const& top() const { return vec[0]; }
T max = std::move(vec[0]);
vec[0] = std::move(vec.back());

Context

StackExchange Code Review Q#97123, answer score: 5

Revisions (0)

No revisions yet.