C++ Data-Oriented Modules using namespaces and extern

Question

I have been coding in a certain style for some time now, where I try to avoid classes and stick to pure data structures whenever possible. Basically every module of my program has its own namespace in which all the data structures and functions are declared.

I started doing this because I wanted to focus on the actual data that gets processed and wanted to abstract data as little as possible. Performance is very important to me and I would like to develop a minimal, cache- and compiler-friendly style and I need someone to help me understand what I actually do by structuring my code like this.

---

So this is how I would structure the functionality of loading 3D meshes into my program for example. The actual code is very abbreviated because right now I would like to only focus on the structure with the namespace and the external declarations.

The header Mesh.h

#include "OpenGL.h" //a higher level namespace containing context information
namespace OpenGL{
    namespace Mesh{

struct Vertex{
    vec3 pos;
    vec3 normal;
    vec2 uv;
};

struct Material{
    vec3 amb;
    vec3 diff;
    vec3 spec;
    float shininess;
};
struct Mesh{
    unsigned int indexOffset;
    unsigned int indexCount;
    unsigned int materialIndex;
};
extern vector<Vertex> allVertices;
extern vector<unsigned int> allIndices;
extern vector<Material> allMaterials;
extern vector<Mesh> allMeshes;

extern string MESH_DIR;
extern vector<string> meshNames; //find mesh index by name


void setMeshDirectory(string pDirectory);

void loadAllMeshes();
Mesh loadMesh(string pFilename);
void addMesh(string pFileName);
    }
}

The code file

#include "Mesh.h"
#define DEFAULT_MESH_DIR "..//assets//meshes//"
//define all external variables
vector<Vertex> allVertices = {};
vector<unsigned int> allIndices = {};
vector<Material> allMaterials = {};
vector<Mesh> allMeshes = {};

string MESH_DIR = DEFAULT_MESH_DIR;
vector<string> meshNames = {};

void OpenGL::Mesh::setMeshDirectory(string pDirectory){
    MESH_DIR = pDirectory;
}

void OpenGL::Mesh::addMesh(string pFileName){
    auto it = find(meshNames.begin(), meshNames.end(), pFileName);
    if(it == meshNames.end()){
        meshNames.push_back(pFileName);
    }else{
    //error
    }
}

void OpenGL::Mesh::loadAllMeshes(){
    allMeshes.resize(meshNames.size()); //resize allMeshes array in advance
    for(unsigned int m = 0; 0 < meshNames.size(); ++m){
        allMeshes[m] = loadMesh(meshNames[m]);
    }
}

OpenGL::Mesh::Mesh OpenGL::Mesh::loadMesh(string pFilename){
    Mesh mesh;
    mesh.indexOffset = allIndices.size();
    unsigned int indexCount;
    unsigned int materialIndex;

    //use assimp to actually load the mesh from an .obj, .3DS, .fbx (etc) file
    //fill allMaterials, allVertices and allIndices with the data from this mesh

    mesh.indexCount = indexCount;
    mesh.materialIndex = materialIndex;
    return mesh;
}

Answer 1

Your header file is missing include guards. Either put #pragma once at the start, or use the more portable:

#ifndef MESH_H_
#define MESH_H_

// ... rest of header

#endif

MESH_H_ has to be unique for the file.

---

namespace OpenGL{

If you don't own OpenGL, I'd recommend not naming your namespace after it. If they release OpenGL in a namespace, your code wouldn't be compatible. Try to use a namespace for your project. Similarly, I'd recommend not naming your header OpenGL.h, unless you put the header in a directory.

---

extern vector<Vertex> allVertices;
extern vector<unsigned int> allIndices;
extern vector<Material> allMaterials;
extern vector<Mesh> allMeshes;

extern string MESH_DIR;
extern vector<string> meshNames; //find mesh index by name

I'm assuming that in OpenGL.h, you have using namespace std;. Don't do this. It is especially bad in a header.

Also, if you find yourself using extern globals, you are almost certainly doing it wrong. These should either not exist at all (as in the calling code takes care of it), or they should be wrapped in a class that the calling code passes around.

These globals and the following point out that you really need to define a class. Maybe something like this:

class MeshManager { // this name is suspicious; "Manager" in a name is usually bad.
    std::vector<Vertex> vertices;
    std::vector<unsigned int> indices;
    std::vector<Material> materials;
    std::vector<Mesh> meshes;

    std::string meshDirectory;
    std::vector<std::string> meshNames;

public:
    void setMeshDirectory(std::string directory);
    void loadAllMeshes();

    Mesh loadMesh(std::string filename);
    void addMesh(std::string filename);
};

Note that this design isn't the best. You still need a constructor, and possibly accessors on the member variables. Also, setMeshDirectory and loadAllMeshes might be better as a constructor to MeshManager, or a static factory function if you prefer that style.

Answer 2

I find your approach of using a namespace instead of a class a good fit for for your purpose. I recently started to use it as an alternative to the standard singleton pattern.

Regarding the global vectors, I would declare them directly in the implementation file, making them accessible by the implementation only: actually, they are an implementation detail. Then you can expose them with proper functions: I'd provide a pair of iterators.

This way you can avoid the (massively documented) pitfalls of using globals, but you are still able to access your data: the fact you're the only one working on this code is not a valid reason to ignore best practices.