Reputation: 101
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:
valueOfSides
pointer in the member initializer of all the constructors for the sake of completion and because I believe it is good practice to initialize the members even though they will get assigned in the body of the constructor. Is this a good practice or a habit? Or should I just give up Initializing valuesOfSides
when it is not necessary to initialize? In this case, only include it in the member initializer of the default constructor and not in the parameterized and copy constructor?Upvotes: 1
Views: 152
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