pimpl class with pointer to base class

[treuss]

I'm trying to implement a class hierarchy and a wrapper class with a pointer to the base class. The base class has operator< overloaded and the implementation makes use of virtual functions as some of the logic for sorting is in the derived classes. Unfortunately, when trying to use the base class operator< from the wrapper, I get a "pure virtual method called".

Below code is meant to illustrate my problem. Unfortunately it crashes on me (upon destruction of vec) and I cannot quite see, why. So two questions:

1. Can somebody spot the error I made in the code below (having lived in Java-land for the last 5 years, I'm sure I just did some stupid error)?
2. How can I implement Wrapper::operator< to use Base::operator<? I know I could write a function and pass it to sort but I'm interessted if there is a way to actually use Base::operator<.

#include <vector>
#include <algorithm>
#include <cmath>

class Base {
public:
  virtual ~Base() {}
  virtual int getValue() const = 0;
  virtual Base * clone() const = 0;
  bool operator<(const Base& rhs) const {
    return getValue() < rhs.getValue();
  }
};

class Derived : public Base {
private:
  int value_;
public:
  Derived() : value_(rand()) {}
  Derived(const Derived& rhs) : value_(rhs.value_) {}
  virtual Derived * clone() const { return new Derived(*this); }
  virtual int getValue() const { return value_; };
};

class Wrapper
{
private:
  Base * pimpl_;
public:
  Wrapper() : pimpl_(new Derived()) {}
  Wrapper(const Wrapper& rhs) : pimpl_(rhs.pimpl_->clone()) {}
  ~Wrapper() { delete pimpl_; }
  bool operator<(const Wrapper& rhs) const {
    return pimpl_->getValue() < rhs.pimpl_->getValue();
  }
};


using namespace std;

int main() {
  vector<Wrapper> vec(10);
  sort(vec.begin(), vec.end());
}

Thanks everybody.

PS: Anybody else hates the new design?

[D_Drmmr]

1. Can somebody spot the error I made in the code below (having lived in Java-land for the last 5 years, I'm sure I just did some stupid error)?

You didn't implement the assignment operator for the Wrapper class.

2. How can I implement Wrapper::operator< to use Base::operator<? I know I could write a function and pass it to sort but I'm interessted if there is a way to actually use Base::operator<.

  bool operator<(const Wrapper& rhs) const {
    return *pimpl_ < *rhs.pimpl_;
  }

[treuss]

You didn't implement the assignment operator for the Wrapper class.

D'oh! Knew it was something stupidly simple.

  bool operator<(const Wrapper& rhs) const {
    return *pimpl_ < *rhs.pimpl_;
  }

Hmm, that's what I assumed and tried but it gave me "the pure virtual method called" error, so I thought I was truncating the derived classes to their base classes. Apparently not though, as it works fine with the test program. Need to check what I did wrong.

Thanks for the help!

[monarch_dodra]

If you have access to it, I'd suggest you define your pimpl_ as type std::unique_ptr<Base>. Ditto for clone.

This could help you in a different places:
1) Clone never leaks.
2) Wrapper class would not have been able to define the operator= you had forgotten to define youself.
3) default destructor works.

Finally, the potentially non-trivial implementation of operator= (if you want one with strong exception safety) becomes easy as pie:

Wrapper& operator=(const Wrapper& rhs)
{
  pimpl_ = rhs.pimpl_->clone();
  return *this;
}

Here, if clone throws, you this object is not yet modified. If it succeeds, the std::unique_ptr:perator= will be the one responsible for the clean up of its own internals. Awesome. Overall, I don't think you could shoot yourself if you tried.

It is a bit of overhead, but I think the extra safety is well worth it.