Passing abstract c++ class to constructor and calling member method

Question

Is it possible to pass an abstract class as an argument and use its member functions as shown below? (Summary: A model is created that needs a solver derived from a base class, and where the system of equations to be solved may change).

#include <iostream>
#include <string>
#include <functional>

class AbstractSolver {
  public:
  virtual ~AbstractSolver(){}
  virtual double solve() = 0;
  virtual void setSystem(std::function<double(double,double)> system) = 0;
};

class ConcreteSolver : public AbstractSolver {
  protected:
    double stepSize;
    double initialValue;
    std::function<double(double, double)> system;
  public:
  ConcreteSolver(double stepSize,
                 double initialValue) :
     stepSize(stepSize),
     initialValue(initialValue) {}

  double solve() {
    // implementation here ...
  }

  void setSystem(std::function<double(double,double)> system) {
    this->system = system;
  }
};

class Model {
  protected:
    std::function<double(double,double,double)> system;
    AbstractSolver * solver;
  public:
    Model(AbstractSolver * solver,
          std::function<double(double, double, double)> system ) {
      this->solver = solver;
      this->system = system;
    }

    ~Model() {
       delete solver;
    }

    double getSolution(double tau) {
      std::function<double(double, double)> sys =
          [this, tau](double t, double y) { return system(t, y, tau); };
      solver->setSystem(sys); // SIGSEGV
      return solver->solve();
    }
};

int main(){
  std::function<double(double, double, double)> system = 
    [](double t, double y, double tau) {
         return 0;// return some mathematical expression
      };
  AbstractSolver * solver = new ConcreteSolver(0, 1);
  Model model = Model(solver, system);
  model.getSolution(0.1);

}

This will compile, but the problem is that it seg faults where I've put the comment above. Can anyone explain why (I wasn't able to find anything with regards to this)? Your suggestions are welcome

Answer 1

To your first question: you can have abstract classes as parameter of methods or constructor - this is the core of the polymorphism. After have said this, off to your problem.

The problem in your code is a double delete/dangling pointer. And as pointed out by others you should adhere to Rule of Three and (better) use smart pointers rather than raw pointers to prevent this.

The problem starts at line:

Model model = Model(solver, system);

With Model(solver, system) a Model-object is created and a copy of it is assigned to model. Now there are two Model-object, but both share the same solver-pointer (because you didn't overwritten the assingment operator, remember the Rule of Three!). When the fist of them goes out of scope, the destructor is called and the pointer solver is destroyed. The second Model-object has now a bad pointer. When the second object goes out of scope, the solver will be freed once again - which ends in undefined behaviour. My compiler is forgiving and I don't see anything.

It is worse, if you dereference the solver-pointer after it was freed once - a segfault is the result. This is not the case in your code, because both objects go out of scope directly after each other at the end of main. Yet you can trigger the segfault by:

  Model model = Model(NULL, system);
  {
     model = Model(solver, system);
  }
  model.getSolution(0.1);

Now, the first model goes out of scope before the call of getSolution because its scope is in between {}. Thereafter the model has a bad pointer and dereferences it somewhere along the call of getSolution.

A better design would be to use smart pointers rather then raw pointer: std::shared_ptr<AbstractSolver> or (depending on your design) std::unique_ptr<AbstractSolver>

With shared pointer your Model class would look like:

class Model {
  protected:
    std::function<double(double,double,double)> system;
    std::shared_ptr<AbstractSolver> solver;
  public:
    Model(const std::shared_ptr<AbstractSolver> &solver,
          std::function<double(double, double, double)> system ) {
      this->solver = solver;
      this->system = system;
    }

   //not needed anymore
   // ~Model() {//nothing to do}

... };

The biggest gain is: you don't have to manage the memory at all and thus cannot make mistakes. Solver pointer is deleted first after the last owner goes out of scope. Due to the Rule of Three you don't need neither copy constructor nor assignment operator, as you don't need destructor.

If you come from java: shared_ptr behaves (almost) exactly as pointers from java.

If your Model owns (opposed to shares) the solver, you should use std::unique_ptr-this way it is much clearer and you can be sure, that your solver isn't shared.

With this you error should go away. But there are some other points you could improve:

a) use rather private than protected members: with protected members the subclasses are tighter coupled with the base class as it would be the case with private members - it is much harder later on to change those.

b) don't use:

Model model = Model(solver, system);

use rather:

Model model(solver, system);

this way you initialize the object directly and don't need any copying, which is faster.