Objects seemingly not being passed by reference in C++

Edit: The problem has been found to be that the Wrapper class is creating copies of the BSCallFunction when passed into the MCEngine function and in the SimulationEngine class shown below. With this in mind I still need to figure out how to make the program do what I want it to, as detailed in the question. The comment section contains the finding thus far.

class SimulationEngine
{
public:
    SimulationEngine(double horizon, const Wrapper<valuationFunction>& theFunction_, RiskFactor simulatedRiskFactor);
    virtual void DoOnePath(double vol, double normvariate) = 0;
    virtual SimulationEngine* clone() const = 0;
    const double GetHorizon();
    Wrapper<valuationFunction>& GetFunction();
    RiskFactor simulatedRiskFactor;
protected:
    double horizon;
    Wrapper<valuationFunction> theFunction;
};

In my main class I'm creating this object:

    BSCallFunction ThisBsCall(nominal, S0, r, d, impvol, TTM, Strike);

Which I'm passing into two classes (derived from the same base class) that will change some of its values (like S0 and r).

    OneStepBSEngine BSSimulation(timeHorizon, zeroDrift, ThisBsCall, RiskFactor::equity);
    OneStepBrownianMotionEngine ShortRateSimulation(timeHorizon, zeroDrift, ThisBsCall, RiskFactor::interest_rate);

I want both the two classes to change the value of the same object so that I end up with a ThisBsCall that has different r and S0.

The classes have this structure:

class OneStepBrownianMotionEngine : public SimulationEngine
{
public:
    OneStepBrownianMotionEngine(double horizon_, double drift_,const Wrapper<valuationFunction>& theFunction_, RiskFactor simulatedRiskFactor_);
    virtual SimulationEngine* clone() const;
    void DoOnePath(double vol, double normvariate);
private:
    double drift;
};

OneStepBrownianMotionEngine::OneStepBrownianMotionEngine(double horizon_, double drift_,const Wrapper<valuationFunction>& theFunction_, RiskFactor simulatedRiskFactor_) : SimulationEngine(horizon_, theFunction_, simulatedRiskFactor_), drift(drift_)
{
}

Where I'm passing the "ThisBsCall" object by const reference.

Back in my main class I will then create the object MCEngine and use the function that will change the values of r and S0 using the Engine classes above, the Enginevector is just a vector of the two Engine classes:

MCEngine VAREngine(EngineVector, covmat);
VAREngine.DoSimulation(gathererCombiner, NumberOfPaths);

I pass the enginevector by const reference as well like so:

MCEngine(const std::vector<Wrapper<SimulationEngine>>& EngineVector, std::vector<std::vector<double>> covMatrix_);

I'm not sure if it matters but the constructor and function used looks like this in the MCEngine class:

MCEngine::MCEngine(const std::vector< Wrapper<SimulationEngine>>& EngineVector_, std::vector<std::vector<double>> covMatrix_)
    : cholMatrix(Cholesky_Decomposition(covMatrix_)), EngineVector(EngineVector_), V(0)
{
}

void MCEngine::DoSimulation(StatisticsMC& TheGatherer, unsigned long NumberOfPaths)
{
    UpdateTTM();  
    double thisPortfolioValue;
    for (unsigned long i = 0; i < NumberOfPaths; ++i)
    {
        std::vector<Wrapper<SimulationEngine>> tmpEngineVector(EngineVector);  //save the origianl EngineVector to use on next path
        //create vector of correlated normal variats with cholezky decompositon
        MJArray corrNormVariates(cholMatrix.size());
        corrNormVariates = 0;
        MJArray NormVariates(cholMatrix.size());
        for (unsigned long i = 0; i < cholMatrix.size(); i++)
        {
            NormVariates[i] = GetOneGaussianByBoxMuller();
            for (unsigned long j = 0; j < cholMatrix[i].size(); j++) {
                corrNormVariates[i] += cholMatrix[i][j] * NormVariates[j];
            }
            corrNormVariates[i] /= cholMatrix[i][i]; //normalize the random variates
        }
        //use each one for simulation with spotvector/volvector
        for (unsigned long j = 0; j < EngineVector.size(); ++j)
        {
            EngineVector[j]->DoOnePath(cholMatrix[j][j], corrNormVariates[j]); //updates the riskfactors for the positions
        }
        ValuePortfolio();
        thisPortfolioValue = GetPortfolioValue();
        TheGatherer.DumpOneResult(thisPortfolioValue);
        EngineVector = tmpEngineVector;
    }
}

Now my problem is when I look at the 2 BSCallFunction objects (which I expect to be the same ThisBsCall) they have different values for S0 and r, so it seems like at some point I copied the object and changed the value of r or S0 of this copy instead of the original and then returned the result. Is it possible to see where this might happen from what I've posted (or general advice on what I might be doing wrong)? I can post more of the code if it helps.

EDIT: The Wrapper class in case that might be what's causing issue:

#ifndef WRAPPER_H
#define WRAPPER_H
template< class T>
class Wrapper
{
public:
    Wrapper()
    {
        DataPtr = 0;
    }
    Wrapper(const T& inner)
    {
        DataPtr = inner.clone();
    }
    Wrapper(T* DataPtr_)
    {
        DataPtr = DataPtr_;
    }
    ~Wrapper()
    {
        if (DataPtr != 0)
            delete DataPtr;
    }
    Wrapper(const Wrapper<T>& original)
    {
        if (original.DataPtr != 0)
            DataPtr = original.DataPtr->clone();
        else
            DataPtr = 0;
    }
    Wrapper& operator=(const Wrapper<T>& original)
    {
    if (this != &original)
    {
    T* newPtr = (original.DataPtr != 0) ?
    original.DataPtr->clone() : 0;
    if (DataPtr != 0)
    delete DataPtr;
    DataPtr = newPtr;
    }
    return *this;
    }

        T& operator*()
    {
        return *DataPtr;
    }
    const T& operator*() const
    {
        return *DataPtr;
    }
    const T* const operator->() const
    {
        return DataPtr;
    }
    T* operator->()
    {
        return DataPtr;
    }
private:
    T* DataPtr;
};
#endif

Solution

You are correct in thinking that the Wrapper class is creating multiple copies of the BSCallFunction object.

For example:

BSCallFunction ThisBsCall(nominal, S0, r, d, impvol, TTM, Strike);
OneStepBrownianMotionEngine ShortRateSimulation(timeHorizon, zeroDrift, ThisBsCall, RiskFactor::interest_rate);

In the line above, a temporary Wrapper object is created and passed into constructor of OneStepBrownianMotionEngine, which makes a clone of the inner object and stores a pointer to it. Then this cloned object is passed to the base class SimulationEngine, which stores a copy of it using the copy constructor of the Wrapper class.

So 1 temporary object + another copy.

This happens likewise for the other engine class OneStepBrownianMotionEngine.

Now my problem is when I look at the 2 BSCallFunction objects (which I expect to be the same ThisBsCall) they have different values for S0 and r, so it seems like at some point I copied the object and changed the value of r or S0 of this copy instead of the original and then returned the result.

They will never be the same because the Wrapper class is designed to clone the object passed to it. You could create the ThisBsCall on the heap (new BSCallFunction(...)) and use the pointer. This will ensure the same ThisBsCall is used by both engines.

However, as you've discovered, this will cause a problem when the engines are destroyed: the destructor of Wrapper tries to delete the same ThisBsCall object. A double delete invariably leads to a crash!

What you what is for your Wrapper to:

use a pointer to ThisBsCall so that you don't make a copy of it
maintain a reference count for each shared owner of a Wrapper object
destroy the ThisBsCall object when the reference count goes down to zero

std::shared_ptr does all this for you and it's part of the standard library. So I would recommend you use this instead of your Wrapper class. It will require the least amount of changes.

Your code could stay the same except you replace instances of

Wrapper<your_class_name>

with

std::shared_ptr<your_class_name>

For example:

std::shared_ptr<valuationFunction> ThisBsCall = std::make_shared<BSCallFunction>(nominal, S0, r, d, impvol, TTM, Strike);

The beauty of smart pointers like std::shared_ptr and std::unique_ptr is that they will take care of the memory management for you. You don't have to worry about manually calling new and delete. All that is done for you in the background. Plus they give you exception safety too. If for whatever reason an exception is thrown during memory allocation, the memory is cleared up for you. For these reasons and more this is why the recommended practice nowadays is to use smart pointers instead of doing the memory management ourselves.

I don't know if you're coding in a multithreaded environment? Although the control block that stores the managed object and the reference counter is thread-safe, a smart pointer per se isn't. You would still have to provide your own thread synchronization.