Question

I have two classes with separate headers: class Renderer and class Texture. The Texture instance is designed to manage some data that resides in the memory pool of a Renderer, and thus a Texture instance cannot exist independently of a Renderer instance. To get a handle to a Texture instance, one simply calls Renderer::loadTexture with the correct filename, and Renderer searches it's database for Texture objects with a matching filename and instantiates a new texture if no matches are found.

//renderer.h
#include "texture.h"
class renderer
{
public:
    Texture* loadTexture(std::string fileName);
private:
    std::map<std::string, Texture*> textureDb;
};

//texture.h
class Texture
{
public:
    Texture(std::string imageFileName);
};

It occurred to me that it would be logical to make the constructors of class Texture private, and declare Texture * Renderer::loadTexture(std::string filename) as a friend of class Renderer:

//renderer.h
#include "texture.h"
class renderer
{
public:
    Texture* loadTexture(std::string fileName);
private:
    std::map<std::string, Texture*> textureDb;
};

//texture.h
class texture;
#include "renderer.h"
class Texture
{
private:
    Texture(std::string imageFileName);
    Texture(const Texture &);
    friend Texture* loadTexture(std::string);
};

However, this only compiles if texture.h is always included before including renderer.h, since class Texture requires class Renderer to be already defined. What would be the best way to prevent this? Include guards are present on both files but are omitted here for brevity.

Was it helpful?

Solution

First and foremost, from the code above, forward-declare where possible. This should help a bit.

// Renderer.h
class Texture;

class Renderer {
   ...
};

// Renderer.cpp
#include "Renderer.h"
#include "Texture.h"

...

// Texture.h
//class Renderer; // looks like this isn't needed

class Texture {
   ...
};

// Texture.cpp
#include "Renderer.h"
#include "Texture.h"

...

With the forward declarations, you should have enough to declare pointers to Texture in the Renderer class header. It looks like that's all you need at this point; push the rest to Renderer.cpp.

Next, avoid friends; however, it might be acceptable in this case.

Finally, circular dependencies are never a good thing. See if you can change your design a bit. See if you can introduce an interface or an abstract base class in order to break your circular dependency. It looks like Renderer shouldn't really need to depend on Texture at all.

Instead, consider rewriting Renderer to depend on some sort of ITexture interface, and then let Texture implement ITexture:

class ITexture
{
   // no data members
   // no method bodies
   // only pure virtual method declarations
};

class Renderer
{
public:
    ITexture* loadTexture(std::string fileName);
private:
    std::map<std::string, ITexture*> textureDb;
};

class Texture : public ITexture
{
   ...
};
Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top