Reputation: 345
I'm currently playing around with the CRTP pattern in my C++ project and stumbled across the problem, that I have to redefine a constructor quiet a few times. I'm curious if my approach is wrong or if I can adjust the code so that I can define it once and all derived classes can handle it just fine.
I have the following code right now:
celestial_body.hpp
:
#pragma once
#include <raylib.h>
#include <memory>
#include <string>
class CelestialBodyBase {
public:
CelestialBodyBase(std::string name, double mass, double radius)
: name(name), mass(mass), radius(radius), position({0.0, 0.0, 0.0}) {}
virtual ~CelestialBodyBase() = default;
virtual void update() = 0;
protected:
std::string name;
double mass;
double radius;
Vector3 position;
};
template <class T>
class CelestialBody : public CelestialBodyBase {
public:
CelestialBody(std::string name, double mass, double radius)
: CelestialBodyBase(name, mass, radius) {}
void update() { derived()->update(); }
private:
T* derived() { return static_cast<T*>(this); }
};
planet.hpp
#pragma once
#include <starsystem/celestial_body.hpp>
class Planet : public CelestialBody<Planet> {
friend class CelestialBody<Planet>;
public:
Planet(std::string name, double mass, double radius);
~Planet();
void update();
};
planet.cpp
#include <starsystem/planet.hpp>
Planet::Planet(std::string name, double mass, double radius)
: CelestialBody(name, mass, radius) {}
Planet::~Planet() {}
void Planet::update() {
// Update planet
// ...
}
As you can see, there is a constructor which is basically the same for all 3 classes being defined here. Mind that I have the CelestialBodyBase
class to make it compatible with STL containers. I kinda expected that I could just define a constructor in for example the base, and the derived classes inherit that. Does the pattern look right this way, can I change something to make it less tedious to type out?
Upvotes: 2
Views: 96
Reputation: 13076
This is how I would model your problem, and that is starting with a pure abstract baseclass. And strong types for Mass/Length etc. So you can't mix up doubles meaning length with doubles meaning mass.
#include <string_view>
#include <string>
#include <memory>
#include <vector>
struct Mass
{
double kg;
};
struct Length
{
double meters;
};
using Radius = Length;
struct Position
{
Length x;
Length y;
Length z;
};
// abstract base class only, is interface this is more flexible than your approach
// since it will allow you to implement mocks/fakse etc too (for dependency injection and unit testing)
class CelestialBodyInterface
{
public:
virtual ~CelestialBodyInterface() = default;
virtual void update() = 0;
// note const and noexcept correctness for getters.
virtual std::string_view get_name() const noexcept = 0;
virtual const Mass& get_mass() const noexcept = 0;
virtual const Radius& get_radius() const noexcept = 0;
virtual const Position& get_position() const noexcept = 0;
};
// Reusable part of the interface implementation
// This is an example of CRTP-mixin (there are actually 2 CRTP patterns)
template<typename Derived>
class CelestialBodyInterfaceImpl : public CelestialBodyInterface
{
public:
CelestialBodyInterfaceImpl(std::string_view name, Mass mass, Radius radius) :
m_name{ name },
m_mass{ mass },
m_radius{ radius }
{
}
virtual std::string_view get_name() const noexcept override
{
return m_name;
}
virtual const Mass& get_mass() const noexcept override
{
return m_mass;
}
virtual const Length& get_radius() const noexcept override
{
return m_radius;
}
virtual const Position& get_position() const noexcept override
{
return m_position;
}
protected:
std::string m_name;
Mass m_mass{};
Length m_radius{};
Position m_position{};
};
// An actual planet class
class Planet : public CelestialBodyInterfaceImpl<Planet>
{
public:
Planet(std::string_view name, Mass mass, Radius radius) :
CelestialBodyInterfaceImpl<Planet>{ name,mass,radius }
{
}
// Probably this can be implementes in CelestialBodyInterfaceImpl
// but maybe you need slightly different rules here.
virtual void update() override
{
m_position.x.meters += 2.0;
}
};
int main()
{
std::vector<std::unique_ptr<CelestialBodyInterface>> solar_system;
solar_system.emplace_back(std::make_unique<Planet>("earth", Mass{ .kg = 5.9722E24 }, Radius{ .meters = 1.5E08 }));
for (const auto& planet : solar_system)
{
planet->update();
}
}
Upvotes: 0
Reputation: 396
Okay, a few items to look at here.
First of all, I don't know if CRTP is actually appropriate here. In this case you are using virtual functions and dynamic dispatch.
Since update is virtual, calling update on a base class pointer (either CelestialBodyBase or CelestialBody) will invoke Planet::update. This is what polymorphism in C++ is about and adding CRTP to the mix is just making things more complex for no good reason.
CRTP becomes more appropriate when the various classes involved do not inherit from each other. (e.g. you have a Planet class and a Basketball class that both have an update member in that case there is likely not a sensible base class for both)
This also means that the CelestialBody class unnecessary as well. Just inherit CelestialBodyBase directly from Planet.
As a side note, it is good form to use the override keyword when overriding a function to explicitly state your intent and cause the compiler to catch future breakages if you refactor the base class.
void update() override; // in Planet
You didn't elaborate on how you are storing things in STL containers. As a general rule you should NOT store CelestialBodyBase objects in a container (although pointers to one or std::shared_ptr is fine) due to object slicing.
Now to your real question: How to eliminate constructor repetition. In the case that you are simply forwarding arguments to the base class constructor, you can lift those constructors with a using statement. If you need to do something more than just invoke the base class constructor you will need something more like what you currently have.
class Planet : public CelestialBodyBase {
public:
using CelestialBodyBase::CelestialBodyBase;
~Planet() = default;
void update() override;
};
Upvotes: 1
Reputation: 63947
I kinda expected that I could just define a constructor in for example the base, and the derived classes inherit that.
The syntax for that is using Base::Base;
. That brings the Base constructor into the current scope.
template <class T>
class CelestialBody : public CelestialBodyBase {
public:
// Base constructor is also valid here.
using CelestialBodyBase::CelestialBodyBase;
void update() { derived()->update(); }
private:
T* derived() { return static_cast<T*>(this); }
};
class Planet : public CelestialBody<Planet> {
friend class CelestialBody<Planet>;
public:
// Base constructor is also valid here.
using CelestialBody::CelestialBody;
~Planet();
void update();
};
Upvotes: 2