nyms1
nyms1

Reputation: 101

Copy constructor's member initializer not requiring a member

This is my class:
(I know currently it violates the rule of three because it doesn't yet have an explicitly defined copy assignment operator.)

#include <iostream>

template <class T>
class die
{
private:
    int sideCount;
    T* valueOfSides;
public:
    die() : sideCount(0), valueOfSides(nullptr) {}
    die(int sideCount, T* valueOfSides) : sideCount(sideCount), valueOfSides(nullptr)
    {
        this->valueOfSides = new T[sideCount];

        for (int i = 0; i < sideCount; ++i)
            this->valueOfSides[i] = valueOfSides[i];
    }

    die(const die& that) : sideCount(that.sideCount), valueOfSides(nullptr) //<- WARNING
    {
        valueOfSides = new T[sideCount];

        for (int i = 0; i < sideCount; ++i)
            valueOfSides[i] = that.valueOfSides[i];
    }

    void printValueOfSides() //Unrelated but I will leave this method here if you decide to use it
    {
        for (int i = 0; i < sideCount; ++i)
            std::cout << "valuesOfSides[" << i << "] = " << valueOfSides[i] << std::endl;
    }

    ~die() { delete[] valueOfSides; }
};

The warning at the copy constructor's initializer list is:
die(const die& that) : sideCount(that.sideCount), valueOfSides(nullptr)<-here The value (I'm guessing nullptr) is never used. When I remove valueOfSides(nullptr) from the copy constructor's initializer list the warning goes away. I know code works without it but for the sake of completion when a die object is created using the copy constructor

int main()
{
    die<int> d1(4, array);
    die<int> d2(d1);

    return 0;
}

I want it first to be initialized with the nullptr then assigned to the values in the constructor's parameter. As its being done with the parameterized constructor.
So my questions are:

Upvotes: 1

Views: 152

Answers (1)

wohlstad
wohlstad

Reputation: 28074

Regarding the warning:
As you can see in @FrançoisAndrieux's comment (and other comments), compiler warning are not necessarity issued for things that are wrong, but rather to any piece of code that seems suspicous. Your compiler determined the initialization to nullptr is superfluous and gave you a legitimate warning. You can chooce whether to remove this initialization.

However:
In C++ it is recomeded not to use old C style arrays.
Instead you can use either std::array for a fixed size array, or std::vector for a dynamic size one (like in your case).

Using std::vector offers a lot of advantages. One of the most important is automatic memory management, so you don't have to manually call new and delete.
Also the constructors and assignment operators of std::vector will allow you to shift from the rule of five to the rule of zero (The rule of three/five/zero).

This is how your code above will look like using std::vector:

#include <iostream>
#include <vector>

template <class T>
class die
{
private:
    std::vector<T> m_valueOfSides;
public:
    die() = default;
    die(std::vector<T> const & valueOfSides) : m_valueOfSides(valueOfSides) {}

    void printValueOfSides() //Unrelated but I will leave this method here if you decide to use it
    {
        for (int i = 0; i < sideCount; ++i)
            std::cout << "valuesOfSides[" << i << "] = " << valueOfSides[i] << std::endl;
    }

    // NOTE: copy ctor and the rest from the rule of five are not needed.
};

int main()
{
    std::vector<int> arr{ 1,2,3,4 };
    die<int> d1(arr);
    die<int> d2(d1);
    return 0;
}

As you can see it became a lot shorter, and less error-prone.

Upvotes: 3

Related Questions