patterncppMinor
D-ary max heap implementation
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.
This are some
#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:
This is unsufficient. You provide some default comparators, which would be fine if my type happened to have
That is easy to understand. And let's actually factor out that loop that I wrote up there. So full rewrite of that function:
I find that much easier to parse. Note also the
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.