0

I'm playing around writing my own heap class. My templated heap class requires the operators '>' and '<' to be defined on the template type.

All seemed to work fine when using an instance of a sample class I wrote (and also worked fine on int). However, since there is so much construction of instances as class instances move from different nodes in the heap, I decided to see what happened when I created a heap of a shared_ptr of my class. While I did see the number of instances constructed go way down, the heap was not working correctly as it appears the smart pointer '>' and '<' are getting called which I guess just compares smart pointer references.

One solution that comes to mind is allowing for a comparison type, just as many of the stl types do, so that I can pass in my own comparison type into the heap class which will dereference the shared_ptr and call the operation on the underlying type.

Some docs I read on the shared_ptr said that they implement the relational operator (namely <) so that they could be used as keys in associative containers. I'm trying to think about when I might want to use the shared_ptr as the key instead of having a specific key of my own.

heap of my sample type which appears to work fine:

heap<foo> foo_heap(heap_type::max);
for (unsigned int i = 0; i < 10; ++i)
    {
    std::string s = "string ";
    s += ('0' + i);
    foo f(i, s);
    foo_heap.push(f);
    }
cout << "root: " << foo_heap.top() << endl;

wrapping my sample class in a shared_ptr which doesn't work, eg. heap constraint not met in terms of what I'm trying to accomplish.

heap<shared_ptr<foo>> foo_heap_smart(heap_type::max);
for (unsigned int i = 0; i < 10; ++i)
    {
    std::string s = "string ";
    s += ('0' + i);
    shared_ptr<foo> f(new foo(i, s));
    foo_heap_smart.push(f);
    }
cout << "root: " << *(foo_heap_smart.top()) << endl;

my sample foo class:

class foo
{
public:
    foo(int value, std::string s) : _value(value), _s(s)
    {
        std::cout << "foo::foo()" << std::endl;
    }

    foo(const foo& f) : _value(f._value), _s(f._s)
    {
        std::cout << "foo::foo(const foo& f)" << std::endl;
    }

    ~foo()
    {
        std::cout << "foo::~foo()" << std::endl;
    }

    virtual void operator=(const foo& f)
    {
        std::cout << "foo::operator=()" << std::endl;
        this->_value = f._value;
        this->_s = f._s;
    }

    virtual bool operator<(const foo& right)
    {
        return this->_value < right._value;
    }

    virtual bool operator>(const foo& right)
    {
        return this->_value > right._value;
    }

    void print(ostream& stm) const
    {
        stm << "value: " << this->_value << ", s: " << this->_s;
    }

private:
    int _value;
    std::string _s;
};

So I assume that many have run into a similar problem. Just wondering what the prescribed solution is. As I mentioned, I think I know of what appears might be a good solution, but wanted to check as it seems that the smart pointer could cause many problems because of their implementation of the relational operators.

Thanks, Nick

  • 1
    The prescribed solution is to provide your own version of the comparison operator if the default one does not suit your need. A better design for your `heap` class would be to take a Comparator type as well, which can default to `std::less` – Arunmu Feb 15 '17 at 18:34
  • 2
    If you're worried about too many copies being made, you should learn about move semantics rather than jumping straight to `std::shared_ptr`. – Miles Budnek Feb 15 '17 at 18:39
  • @miles, I would love to learn about move semantics. Can you point me to some docs? I was wondering whether there was some special swap I could use that would bypass the constructor/destructor. –  Feb 15 '17 at 19:17
  • 1
    See [What are move semantics?](http://stackoverflow.com/questions/3106110/what-are-move-semantics) The answers on that question are quite in-depth. – Miles Budnek Feb 15 '17 at 21:16

1 Answers1

0

The prescribed solution is to provide your own version of the comparison operator if the default one does not suit your need. A better design for your heap class would be to take a Comparator type as well, which can default to std::less.

template <typename T, typename Comp = std::less<T>>
class heap {
...
};

And now provide you own version of less specialized for shared_ptr.

template <typename T>
struct less<shared_ptr<T>> {
    bool operator()(const shared_ptr<T>& a, const shared_ptr<T>& b) const {
      *a < *b;
    }
};

For a better design, you cann add some metaprogramming hack to make it work only for type T which can be compared.

Arunmu
  • 6,837
  • 1
  • 24
  • 46
  • thanks. My heap implementation uses both '>' and '<' so just providing less is probably not going to be sufficient. By the way, I was planning to allow support for a comparison type as the stl classes do. –  Feb 15 '17 at 19:19
  • just want to make sure I'm understanding what exactly you're saying. You mention both comparison operator and Comparator type. Is the operator just the operator() on the comparison type? Also, since I use both '<' and '>' I'm wondering whether it would be better to instead required a comparison operator as opposed to a predicate so that I can only require a single optional Comparator type instead of two. –  Feb 16 '17 at 02:24
  • @nickdu Using `operator <` or a Comparator like above is basically a choice. I prefer a Comparator as that can be passed as a type in template parameters and this is what the standard library also does. – Arunmu Feb 16 '17 at 05:17