Search code examples
c++includeforward-declaration

Should all objects in a class be a reference?


Is it bad practice to always make my object member's data type a reference?

After reading this question about avoiding #include I've have been trying to avoid using #include in my current C++ project and always forward declare my data types.

Here is an example code snippet from my project. In the header I have EnemyBuilder &m_builder; and then I initialize it in the source.

EnemyFactory.h

#include <string>

#ifndef ENEMYFACTORY
#define ENEMYFACTORY

class World;
class Enemy;
class EnemyBuilder;

class EnemyFactory
{
private:
    EnemyBuilder &m_builder;
    World &m_world;

public:
    EnemyFactory(World &world);

    Enemy* produce(std::string type);
};

#endif

EnemyFactory.cpp

#include "EnemyFactory.h"

#include "EnemyBuilder.h"
#include "World.h"
#include "Enemy.h"

EnemyFactory::EnemyFactory(World &world) :
    m_world(world), m_builder(EnemyBuilder())
{

}

Enemy* EnemyFactory::produce(std::string type)
{
    Enemy *product = new Enemy(m_world);

    if (type == "basic")
    {
        product->setSpeed(5000.0f);
    }

    return product;
}

Solution

  • Is it bad practice to always make my object member's data type a reference?

    Not only is it bad practice, it's inefficient and restrictive.

    Why?

    1. Objects of classes that contain references are automatically non-assignable. The standard has std::reference_wrapper<> (basically a wrapped pointer) to get around this restriction when binding reference types to function objects.

    2. You are obliged to manage object lifetimes. Since you're storing references rather than objects, your class has no possibility to clean up after itself. In addition, it can't ensure that the objects to which it is referring are still there. You've just lost all the protection that c++ gives you. Welcome to the stone age.

    3. It's less efficient. All object access now gets dereferenced through what is essentially a pointer. And you will most-likely lose cache locality. Pointless.

    4. you lose the ability to treat objects as values. This will results in less-efficient, convoluted programs.

    5. I could go on...

    What should I do?

    Prefer value-semantics (treating objects as values). Let them be copied (most of the copies will be elided anyway if you write your classes nicely). Encapsulate and compose them into larger objects. Let the container manage the lifetime of the components. That's its job.

    What about the time cost of including a few header files?

    Unless you're doing hideously complex template expansion across hundreds (thousands) of template classes, it's unlikely to even approach being a problem. You waste more time making the coffee than you will compiling a few source files.