Non-pointer class member: how good is it?

Question

I have a non-pointer class member that I need to initialize in the constructor:

class Alerter {

protected:
  Timer timer;

public:
  Alerter(int interval);
};

and then

Alerter::Alerter(int interval) {
    timer = createTimer(interval);
}

(simplified code just to demonstrate the problem).

I have some doubts and concerns that timer probably is first created using its parameter-less constructor and later that instance is overwritten by the contents that the createTimer function returns.

How good is the approach? The possible answers could be:

The "empty timer" created by its parameter-less constructor is not actually created, as the compiler is smart enough to find we have never referenced it before overwriting the value.
The empty timer is created, but this is OK as well-written code should support a cheap parameter-less constructor for throwaway instances.
It's better to use pointers.

Which of these assumptions (or maybe something else) is the most correct?

Can't you use a constructor initializer list to construct `timer`? — Some programmer dude, May 31 '18 at 09:21
If you have appropriate constructor in `Timer` class you can just `Alerter::Alerter(int interval) : timer{ interval }{}` — Killzone Kid, May 31 '18 at 09:28

score 11 · Accepted Answer · answered May 31 '18 at 09:22

11

timer is first default-constructed and then assigned. Of course you can make assumption about how cheap a Timer is to default-construct or about compiler optimization, but here you don't need this since can prevent default-construction by using an initializer list:

Alerter::Alerter(int interval) : timer(createTimer(interval)) { }

This will work, unless your Timer class is copy-assignable but not copy-constructible, which would be weird.

answered May 31 '18 at 09:22

Holt

36,600
7
92
139

3

default construction could also be prevented by declaring the default ctor `deleted`. This would enforce the use of an initializer list – clickMe May 31 '18 at 09:29
1

"move-constructible" is sufficient since C++11 anyway, and neither is required starting from C++17 due to (a lack of) temporary materialization. – Arne Vogel May 31 '18 at 12:37

Tibor Takács · Answer 2 · 2018-06-01T08:38:03.587

Both constructor will be called (and then the assignment as well).

I have extended your example:

class Timer
{
  public:
    Timer() : m_interval(0)
    {
        std::cout << "Constructor withOUT parameter has been called." << std::endl;
    }

    Timer(int interval) : m_interval(interval)
    {
        std::cout << "Constructor with parameter has been called." << std::endl;
    }

    Timer(const Timer& timer)
    {
        std::cout << "Copy constructor has been called." << std::endl;
        m_interval = timer.m_interval;
    }

    Timer(Timer&& timer)
    {
        std::cout << "Move constructor has been called." << std::endl;
        m_interval = timer.m_interval;
    }

    void operator=(Timer&& timer)
    {
        std::cout << "Move assignment has been called." << std::endl;
        m_interval = timer.m_interval;
    }

  private:
    int m_interval;
};

class Alerter
{
  protected:
    Timer timer;

  public:
    Alerter(int interval)
    {
        timer = Timer(interval);
    }
};

... 
Alerter alerter(12);
...

Output:

Constructor withOUT parameter has been called.
Constructor with parameter has been called.
Move assignment has been called.

And answering your question. It is better if the member variables are initialized only once but this is not a hard rule. For example, if the Timer is a very complex object but the default constructor creates just a dummy object, it is not too expensive and acceptable.

So, if you can avoid, better to avoid. Using pointer is a possible solution, but note the followings:

Better to use std::unique_ptr to avoid memory leaks.
This surely will require a heap memory allocation operation. If the objects are anyway on the heap (e.g. uses complex internal structures, like STL containers), it is not a problem. But if the objects are quite simple and the main use case is to create them as local objects, this can cause quite a big performance penalty. This should be a conscious design decision.

Non-pointer class member: how good is it?

2 Answers2