1

I have a class and the constructor accepts a parameter. For example,

class Entity
{
private:
    int number_;
public:
    Entity(int number):number_(number)
    {
       std::cout << "Entity object created";
    }
}

// header
class SuperEntity
{
private:
   Entity *entity_;

public:
   SuperEntity(int value);
};

// source
SuperEntity::SuperEntity(int value)
{
    entity_ = new Entity(value);
}

class SuperEntity has a private member Entity. Since in order to instantiate Entity you need to pass in an int to it's constructor and cannot be done the declaration file (superentity.h) because the int value needed to instantiate Entity is not available yet, is it okay to dynamically allocate Entity in SuperEntity's constructor? Is this a bad practice? Thanks.

Vino
  • 2,111
  • 4
  • 22
  • 42
  • 4
    Sure, but you should consider using `std::unique_ptr` in this case instead of `Entity*`. – François Andrieux Jan 01 '18 at 02:39
  • 1
    It seems you are unaware of the *member initializer list*: just make `Entity` a normal member and initialize it in `SuperEntity`'s constructor: `SuperEntity(int value): entity(value) { /*...*/ }` – Dietmar Kühl Jan 01 '18 at 02:42
  • @AlexanderHuszagh: it seems easier to use `std::optional` than faffing about with `std::aligned_storage` and `reinterpret_cast`s. Even if you don't want to spent an extra flag you could use a `union` without anything funky. – Dietmar Kühl Jan 01 '18 at 02:46
  • @DietmarKühl Wait did I misread this. My bad. – Alex Huszagh Jan 01 '18 at 02:58
  • @DietmarKühl I knew about member initializer list but didn't know it could be used to initialise object this way. Thank you very much! I learnt a lot. – Vino Jan 01 '18 at 03:52

3 Answers3

4

It is ok per the language but not necessarily the best pratice.

  1. Use an object if you can.
  2. Failing that, use a smart pointer instead of a raw pointer. See std::shared_ptr and std::unique_ptr.
  3. If you must use a raw pointer, make sure to follow The Rule of Three.
R Sahu
  • 204,454
  • 14
  • 159
  • 270
4

As Dietmar remarked, use a member initializer list:

class SuperEntity
{
    Entity entity_;

public:
    SuperEntity( int value )
        : entity_{ value }
    {}
};
Cheers and hth. - Alf
  • 142,714
  • 15
  • 209
  • 331
  • Thank you. I always used member initialiser to initialise member variables but not objects. My fundamental is not so strong. – Vino Jan 01 '18 at 03:54
0

It's fine to have a pointer data field for a decoupled has-a relationship. If you really want to stick to pointers then you should prefer the std::unique_ptr to raw pointer and utilize the std::make_unique function in the member initializer list of the constructor:

class SuperEntity {
private:
    std::unique_ptr<Entity> entity_;
public:
    SuperEntity(int value);
};
SuperEntity::SuperEntity(int value)
    : entity_(std::make_unique<Entity>(value))
{}

If you want to have a classic has-a relationship abstraction, namely a data field whose lifetime is bound to owners lifetime then loose the pointer and go with the object:

class SuperEntity {
    private:
        Entity entity_;
    public:
        SuperEntity(int value);
    };
    SuperEntity::SuperEntity(int value)
        : entity_(value)
    {}
};

The entity_ object will be destroyed once the object of type SuperEntity goes out of scope.

Ron
  • 14,674
  • 4
  • 34
  • 47