Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Is it ok to dynamic cast "this" as a return value?

This is more of a design question.

I have a template class, and I want to add extra methods to it depending on the template type. To practice the DRY principle, I have come up with this pattern (definitions intentionally omitted):

template <class T>
class BaseVector: public boost::array<T, 3>
{
protected:
    BaseVector<T>(const T x, const T y, const T z);
public:
    bool operator == (const Vector<T> &other) const;
    Vector<T> operator + (const Vector<T> &other) const;    
    Vector<T> operator - (const Vector<T> &other) const;
    Vector<T> &operator += (const Vector<T> &other)
    {
        (*this)[0] += other[0];
        (*this)[1] += other[1];
        (*this)[2] += other[2];

        return *dynamic_cast<Vector<T> * const>(this);
    }

    virtual ~BaseVector<T>()
    {
    }
}

template <class T>
class Vector : public BaseVector<T>
{
public:
    Vector<T>(const T x, const T y, const T z)
    : BaseVector<T>(x, y, z)
    {
    }
};

template <>
class Vector<double> : public BaseVector<double>
{
public:
    Vector<double>(const double x, const double y, const double z);
    Vector<double>(const Vector<int> &other);
    double norm() const;
};

I intend BaseVector to be nothing more than an implementation detail. This works, but I am concerned about operator+=. My question is: is the dynamic cast of the this pointer a code smell? Is there a better way to achieve what I am trying to do (avoid code duplication, and unnecessary casts in the user code)? Or am I safe since, the BaseVector constructor is private?

EDIT:

Sorry, yes I have virtual dtor, but I forgot to paste it, the code doesn't compile without it.

like image 975
Panayiotis Karabassis Avatar asked Dec 15 '22 22:12

Panayiotis Karabassis


1 Answers

I would recommend that you consider an alternative approach (note that in the below examples, I have simplified the code to the bare minimum required to demonstrate the alternative approaches).

First, consider the Curiously Recurring Template Parameter (CRTP):

template <typename T, typename DerivedVector>
struct BaseVector
{
    DerivedVector& operator+=(DerivedVector const& other)
    {
        return static_cast<DerivedVector&>(*this);
    }
};

template <typename T>
struct Vector : BaseVector<T, Vector<T>>
{
};

Since you always know what the derived type is, a static_cast is sufficient. If Vector<T> is the only class whose base is BaseVector<T> and if you are guaranteed that the T parameters are always the same, then strictly speaking, the CRTP parameter is unnecessary: you always know what the derived type is, so a static_cast is safe.

Alternatively, consider that operators need not be member functions, so you could declare non-member operator function templates:

template <typename T, typename DerivedVector>
struct BaseVector
{
};

template <typename T>
struct Vector : BaseVector<T, Vector<T>>
{
};

template <typename T>
Vector<T>& operator+=(Vector<T>& self, Vector<T> const& other)
{
    return self;
}

While the latter is preferable for operators, it won't work for ordinary, nonoperator member functions, so the CRTP approach would be preferable for those.

like image 132
James McNellis Avatar answered Dec 30 '22 15:12

James McNellis