Calling a static method from constructor's member initializer list

Question

I'm implementing a simple rendering system for a game engine. In my engine I have renderable entities that have a Model component (I'm using inheritance as opposed to a ECS for my engine for now, but encapsulating common behaviour into components will make things easier when/if I want to transistion to a component based architecture). This Model class contains a mesh (vertex buffers and index buffers), textures and materials.

Some entities' models are loaded from files (3D models generated by modeling applications), so I just pass a Model into their constructors, while some others have mesh and vertex attributes that are "hard wired" into their class. For example a SkyBox class would be coded like this:

// SkyBox.hpp

class SkyBox 
{
public:
    SkyBox(const wchar_t *cubeMapFilePath);
    ~SkyBox();
    XMFLOAT4X4 const &GetWorldMatrix() const;
    Model const &GetModel() const;
    void SetRotation(XMFLOAT3 const &rotation);
    static Model CreateSkyBox(const wchar_t *cubeMapFilePath);   // static member function called from constructor
private: 
    Model mModel;                                                // the member with no def-ctor
    XMFLOAT3 mRotation;
    mutable XMFLOAT4X4 mWorldMatrix;
    mutable bool mDirtyFlag = true;
};


// SkyBox.cpp

#include "SkyBox.h"

SkyBox::SkyBox(const wchar_t *cubeMapFilePath) : mModel(CreateSkyBox(cubeMapFilePath)), mRotation(0.0f, 0.0f, 0.0f)
{
}

SkyBox::~SkyBox()
{
}

Model SkyBox::CreateSkyBox(const wchar_t *cubeMapFilePath)
{
    Mesh mesh;
    std::vector<XMFLOAT3> positions;
    positions.push_back(XMFLOAT3(-0.5f, 0.5f, -0.5f));
    positions.push_back(XMFLOAT3(0.5f, 0.5f, -0.5f));
    positions.push_back(XMFLOAT3(0.5f, -0.5f, -0.5f));
    positions.push_back(XMFLOAT3(-0.5f, -0.5f, -0.5f));
    positions.push_back(XMFLOAT3(-0.5f, 0.5f, 0.5f));
    positions.push_back(XMFLOAT3(0.5f, 0.5f, 0.5f));
    positions.push_back(XMFLOAT3(0.5f, -0.5f, 0.5f));
    positions.push_back(XMFLOAT3(-0.5f, -0.5f, 0.5f));

    std::vector<unsigned int> indices{ 0, 1, 3, 3, 1, 2, 5, 4, 6, 6, 4, 7, 4, 0, 7, 7, 0, 3, 1, 5, 2, 2, 5, 6, 4, 5, 0, 0, 5, 1, 3, 2, 7, 7, 2, 6 };

    mesh.LoadAttribute("POSITION", &positions[0], positions.size());
    mesh.LoadIndexBuffer(indices);

    std::vector<Texture> cubeMap;
    Texture texture(Texture::CUBE_MAP);
    texture.LoadCubeMap(cubeMapFilePath);
    cubeMap.push_back(texture);

    Material material = {};

    return Model(mesh, cubeMap, material);
}

//    .... other members

My Model class doesn't have a default constructor, so when I define a class that has a model component I must provide a way to construct one in the entity's class constructor, but to construct a Model I need first to perform some calculations (compute vertex positions, normals ecc..).

I came up with this solution, calling a (private) static member function that performs the necessary calculations and returns a Model, with which to initialize the Model member in the class. An alternative would be to give the Model class a default constructor and perform the calculations in the entity's constructor and then assign to the Model member, but it doesn't make much sense to have an empty model.

So, is this solution an hack or is it good, or is there a pattern for this kind of issues, or should I go for the default constructor?

Actually, I had to give the Model class a default constructor because I need to access data members during the construction of models for other classes (a Terrain class, for example, stores the heights of the mesh grid in a data member array for collision detection). I don't know if it's the right way to do it but it's quick and dirty and it works.

Answer 1

You said in your text that CreateSkyBox () was private, but its not declared private.

There is absolutely nothing wrong with having static 'helper' functions in a class, which are used to help with whatever makes sense - including creating auxiliary objects like the Model.

If it never makes sense to have an empty Model, then it should not have a no-argument constructor (exactly as you've done).

Though I think your code / approach is fine, there is another pattern you can use for similar circumstances. If you want to have mModel in an unconstructed (null) state - use optional instead of Model; Then you can construct it inside the body of your constructor. In your particular case, I believe what you've done is better, but if your needs change, and you need to be able to construct some objects (like Skybox) when you dont have all the data yet (when reading objects from a file is a common case where this can happen) - then using std::optional can help there.

Answer 2

It may be open to question whether it's actually better, but another possibility to consider would be whether you can equip Model with a constructor that builds the Model appropriately, then inject that dependency into the SkyBox:

class Model {
public:
    Model(Mesh const &mesh, Texture const &texture, Material const &mat);
};

class SkyBox {
public:
    SkyBox(Model const &m) : mModel(Model(input_data)) {}
    // ...
};

I'm certainly not going to claim that this is necessarily the right way to do things. In particular, if a Model is primarily an aggregation of a bunch of data, this may not be useful at all--but there are certainly cases where it can be a useful.