1
\$\begingroup\$

Any suggestions on how the implementation of the C++ Adapter pattern could be improved here? Implemented via:

  1. Composition
  2. Inheritance

What needed to be adapted was the OptionLegacy class which returns implied volatility of the option but the client needs the dollar price so a simple conversion was included.

#include <iostream>
#include <cmath>

// Return price of option by volatility
// Adaptee - old legacy type option objects
class OptionLegacy
{
protected:
    float volatilityPrc;
    double spotPrice;
    int expiryDays;

public:
    OptionLegacy(float v, double s, int t) :
        volatilityPrc(v), spotPrice(s), expiryDays(t) {}

    float GetVol() const {
        return volatilityPrc;
    }
    double GetSpot() const {
        return spotPrice;
    }
    int GetTau() const {
        return expiryDays;
    }
};


// Return price of option in dollars. Current interface that client uses
class OptionPriceDollars
{
protected:
    float vol;
    double spot;
    double tau;

public:
    OptionPriceDollars() = default;
    OptionPriceDollars(float v, double s, double t) :
        vol(v), spot(s), tau(t) {}

    virtual float GetPrice() {
        return 0.4 * spot * vol * sqrt(tau);
    }
};

// via composition
class Adapter
{
    OptionLegacy adaptee;

public:
    Adapter(const OptionLegacy & o):  adaptee(o) {}

    float GetPrice() {
        return 0.4 * adaptee.GetSpot() * adaptee.GetVol() / 100 * sqrt(adaptee.GetTau()/365);
    }
};

// via inheritance
class AdapterInheritance : public OptionPriceDollars, private OptionLegacy
{

public:
    AdapterInheritance(const OptionLegacy & o) : OptionLegacy(o.GetVol(), o.GetSpot(), o.GetTau()) {};

    float GetPrice() override {
        return 0.4 * spotPrice * volatilityPrc / 100 * expiryDays / 365;
    }

};


int main()
{
    OptionPriceDollars od(0.14, 100, 1);
    std::cout << "The price of the option is " << od.GetPrice() << std::endl;

    OptionLegacy op(14, 100, 365);
    Adapter a(op);
    std::cout << "The price of the old style option is (composition) " << a.GetPrice() << std::endl;

    AdapterInheritance b(op);
    std::cout << "The price of the old style option is (inheritance) " << b.GetPrice() << std::endl;

    int x;
    std::cin >> x;
}
\$\endgroup\$

1 Answer 1

2
\$\begingroup\$

Incorrect use of inheritance

There is a problem with class AdapterInheritance. It contains both the member variables of OptionLegacy and of OptionPriceDollars. But only one set of variables is desired; those of OptionLegacy. You should not inherit from the concrete OptionPriceDollars class. If anything, you should only inherit from an "interface", however C++ does not have interface classes like some other programming languages. You could use pure virtual functions to create an abstract base class, but that's not exactly the same. So I recommend you just write:

class AdapterInheritance : private OptionLegacy
{
public:
    AdapterInheritance(const OptionLegacy & o) : OptionLegacy(o.GetVol(), o.GetSpot(), o.GetTau()) {};

    float GetPrice() {
        return 0.4 * spotPrice * volatilityPrc / 100 * expiryDays / 365;
    }
};

Or if you need to use a base class, create an abstract one:

class OptionPrice {
public:
    virtual float GetPrice() = 0;
};

And have both AdapterInheritance and OptionPriceDollars publicly inherit from that.

Other ways to create adaptable interfaces

C++ is a multi-paradigm language. You don't have to put everything into classes. You could create a freestanding function GetPrice() that takes a reference to any object, and calls the member function GetPrice():

template<typename T>
float GetPrice(T& o) {
    return o.GetPrice();
}

That would then work on OptionPriceDollars as well as your adapters. To make it also work with OptionLegacy, you can provide an overload of GetPrice():

float GetPrice(OptionLegacy& adaptee) {
    return 0.4 * adaptee.GetSpot() * adaptee.GetVol() / 100 * sqrt(adaptee.GetTau()/365);
}

That way, in main() you can write:

std::cout << "The price of the option is " << GetPrice(od) << '\n';
std::cout << "The price of the old style option is " << GetPrice(op) << '\n';

Be consistent

I see different variable names for exactly the same thing, like volatilityPrc, vol and v. I see both float and double being used, if the result of GetPrice() is going to be a float, why use doubles? Why does OptionPriceDollars have a default constructor but the other classes don't?

Prefer \n over std::endl

Prefer using '\n' instead of std::endl; the latter is equivalent to the former, but also forces the output to be flushed, which is usually not necessary and might impact performance.

Improve the constructor of AdapterInheritance

You can make use of the fact that OptionLegacy has an implicit copy constructor, and simplify the constructor of AdapterInheritance like so:

AdapterInheritance(const OptionLegacy & o) : OptionLegacy(o) {}

That's actually very much what you did in Adapter. Another way to implement the constructor is to have it accept any set of arguments, and pass them on to the constructor of OptionLegacy:

template<typename... Args>
AdapterInheritance(Args&&... args): OptionLegacy(std::forward<Args>(args)...) {}

Which might look weird if you have never used templates parameter packs and perfect forwarding before, but this is a standard way to do this. The advantage is that you only have to write this one constructor, and it will cover all the possible constructors OptionLegacy has, including the implicit copy and move constructors. Another advantage of this is that you no longer have to create an OptionLegacy first, you can now just write:

AdapterInheritance b(14, 100, 365);
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.