Skip to main content
added 2 characters in body; edited tags; edited title
Source Link
Jamal
  • 34.9k
  • 13
  • 133
  • 237

Review of design and style Implementation of thisbinary heap in C++ implementation of Binary Heap

Things I can think of things likeinclude integer overflow, if the input type is int, etc. But other than that, what do you think is wrong with this code, design wise-wise, style wise -wise, and also in terms of other edge/corner cases.?

binaryheap.hbinaryheap.h

binaryheap.cppbinaryheap.cpp

Finally, binaryheap_gtest.cppbinaryheap_gtest.cpp

Review of design and style of this C++ implementation of Binary Heap

Things I can think of things like integer overflow, if the input type is int etc. But other than that, what do you think is wrong with this code, design wise, style wise , also in terms of other edge/corner cases.

binaryheap.h

binaryheap.cpp

Finally, binaryheap_gtest.cpp

Implementation of binary heap in C++

Things I can think of include integer overflow, if the input type is int, etc. But other than that, what do you think is wrong with this code, design-wise, style-wise, and also in terms of other edge/corner cases?

binaryheap.h

binaryheap.cpp

binaryheap_gtest.cpp

Source Link
sumodds
  • 253
  • 1
  • 2
  • 6

Review of design and style of this C++ implementation of Binary Heap

Things I can think of things like integer overflow, if the input type is int etc. But other than that, what do you think is wrong with this code, design wise, style wise , also in terms of other edge/corner cases.

binaryheap.h

#ifndef INCLUDE_BINARYHEAP_H_
#define INCLUDE_BINARYHEAP_H_

#include <vector>
#include <iterator>
#include <cmath>

// Assume min heap
template <class T>
class BinaryHeap
{
 private:
  unsigned long heap_size_;
  std::vector<T> data_;
  // typedef typename std::vector<T>size_type heap_size_;
  void SiftUp(unsigned long node);
  void SiftDown(unsigned long node);

 public:
  BinaryHeap(unsigned long num_elements);
  BinaryHeap();
  ~BinaryHeap() {}

  // Misc Operations (using STL namimg).
  unsigned long count() { return heap_size_;}  // Get count of objects.
  void clear();                      // clear the object for reuse.

  // Main operations allowed by the data structure.
  template <class I>
  int Heapify(I start, I end);
  const T FindXtrma();
  const T ExtractXtrma();
  void Insert(const T& data);   // Insert(key)
  void Delete(unsigned long element);     // Delete(element)
  void IncreaseKey(unsigned long element, const T& change);
  void DecreaseKey(unsigned long element, const T& change);
  unsigned long get_size(){return(heap_size_);}
  /* void Merge(class &Heap); */
};

#endif  // INCLUDE_BINARYHEAP_H_

binaryheap.cpp

#include <binaryheap.h>
#include <vector>
#include <algorithm>

template <class T>
BinaryHeap<T>::BinaryHeap(unsigned long num_elements)
{ heap_size_ = num_elements;
  data_.reserve(num_elements);
}


template <class T>
BinaryHeap<T>::BinaryHeap()
{ heap_size_ = 0;
}

template <class T>
void BinaryHeap<T>::SiftDown(unsigned long node)
{ unsigned long lchild = 2*node+1;
  unsigned long rchild = 2*node+2;

  bool rexists = rchild < heap_size_;
  bool lexists = lchild < heap_size_;
  if (!lexists && !rexists) return;
  bool left_small;
  if (rexists && data_[lchild] > data_[rchild])
    left_small = false;
  else
    left_small = true;

  if (data_[lchild] < data_[node] && left_small)
  { std::swap(data_[node], data_[lchild]);
    SiftDown(lchild);
  }
  else if (data_[rchild] < data_[node] && rexists && !left_small)
  { std::swap(data_[node], data_[rchild]);
    SiftDown(rchild);
  }
}

template <class T>
void BinaryHeap<T>::SiftUp(unsigned long node)
{ long parent = floor(node/2)-(node+1)%2;
  bool pexists = parent >= 0;
  if (pexists && data_[parent] > data_[node])
  { std::swap(data_[parent], data_[node]);
    SiftUp(parent);
  }
}

template <class T>
template <class I>
int BinaryHeap<T>::Heapify(I start, I end)
{ unsigned long d = std::distance(start, end);
  if (data_.size() != 0)
    this->clear();
  if (heap_size_ == 0)
    heap_size_ = d;
  // may be warn them.
  if (d != heap_size_)
    heap_size_ = d;
  for (I i = start; i != end; ++i)
    data_.push_back(*i);
  for (unsigned long i = heap_size_-1; i <= heap_size_; --i)
  { SiftDown(i);
  }
  return 0;
}

template <class T>
const T BinaryHeap<T>::FindXtrma()
{ if (heap_size_ <= 0)
    return ((T)(0));
  return data_.front();
  // return this->data_[0];
}

template <class T>
const T BinaryHeap<T>::ExtractXtrma()
{ if (heap_size_ <= 0)
    return ((T)(0));
  T max_value = data_.front();
  std::swap(data_.front(), data_.back());
  data_.pop_back();
  --heap_size_;
  SiftDown(0);
  return max_value;
}

template <class T>
void BinaryHeap<T>::Insert(const T& new_node)
{ data_.push_back(new_node);
  SiftUp(data_.size()-1);
  ++heap_size_;
}

template <class T>
void BinaryHeap<T>::Delete(unsigned long element)
{ if (element >= heap_size_)
    return;
  std::swap(data_[element], data_.back());
  data_.pop_back();
  --heap_size_;
  SiftUp(element);
  SiftDown(element);
}

template <class T>
void BinaryHeap<T>::IncreaseKey(unsigned long element, const T& change)
{ data_[element] = data_[element]+change;
  SiftDown(element);
}

template <class T>
void BinaryHeap<T>::DecreaseKey(unsigned long element, const T& change)
{ data_[element] = data_[element]-change;
  SiftUp(element);
}

template <class T>
void BinaryHeap<T>::clear()
{ if (data_.size()  > 0)
    data_.clear();
}

template class BinaryHeap<int>;
template class BinaryHeap<float>;
template class BinaryHeap<unsigned int>;
template class BinaryHeap<long>;


template int BinaryHeap<int>::Heapify(std::vector<int>::iterator, std::vector<int>::iterator);
template int BinaryHeap<float>::Heapify(std::vector<float>::iterator, std::vector<float>::iterator);
template int BinaryHeap<unsigned int>::Heapify(std::vector<unsigned int>::iterator, std::vector<unsigned int>::iterator);
template int BinaryHeap<long>::Heapify(std::vector<long>::iterator, std::vector<long>::iterator);

Finally, binaryheap_gtest.cpp

#include <binaryheap.h>
#include <vector>
#include "gtest/gtest.h"

using namespace std;

namespace
{ template<class T>
  class BHeapTest : public ::testing::Test
  {
   public:
    BinaryHeap<T> b1;
    BHeapTest() { b1 = BinaryHeap<T>(1000);}
    virtual ~BHeapTest() {}

   protected:
    vector<T> data_;
    virtual void SetUp()
    {
      unsigned long max_val = 2000;
      for (T i = 1000; i < (T)max_val; ++i)
        data_.push_back(i);
      b1.Heapify(data_.begin(), data_.end());
    }
    virtual void TearDown()
    { data_.clear();
    }
  };

  typedef ::testing::Types<int, unsigned int, long> MyTypes;
  TYPED_TEST_CASE(BHeapTest, MyTypes);

  TYPED_TEST(BHeapTest, SimpleTest)
  { EXPECT_EQ(1000, this->b1.FindXtrma());
    EXPECT_EQ(1000, this->b1.get_size());
    this->b1.Insert(3000);
    EXPECT_EQ(1000, this->b1.FindXtrma());
    EXPECT_EQ(1001, this->b1.get_size());
    this->b1.Delete(0);
    EXPECT_EQ(1001, this->b1.FindXtrma());
    EXPECT_EQ(1000, this->b1.get_size());
    this->b1.DecreaseKey(999, 1000);
    EXPECT_EQ(999, this->b1.FindXtrma());
    EXPECT_EQ(1000, this->b1.get_size());
    this->b1.IncreaseKey(0,1000);
    EXPECT_EQ(1001, this->b1.FindXtrma());
    EXPECT_EQ(1000, this->b1.get_size());
  }

  TYPED_TEST(BHeapTest, ComplexTest)
  { EXPECT_EQ(1000, this->b1.FindXtrma());
    EXPECT_EQ(1000, this->b1.get_size());
    for (int i = 0; i < 10; ++i)
      this->b1.ExtractXtrma();
    EXPECT_EQ(1010, this->b1.FindXtrma());
    EXPECT_EQ(990, this->b1.get_size());
    for (int i = 0; i < 10; ++i)
      this->b1.ExtractXtrma();
    EXPECT_EQ(1020, this->b1.FindXtrma());
    EXPECT_EQ(980, this->b1.get_size());
    this->b1.Insert(3232);
    EXPECT_EQ(981, this->b1.get_size());
  }
}  //  namespace

int main(int argc, char **argv)
{ ::testing::InitGoogleTest(&argc, argv);
  return RUN_ALL_TESTS();
}