I have the class below and tried to add copy and move constructor and assignment operator. My goal is to have least amount of copy and be as optimized as possible.
I expect the vectors to be filled in place, that is no copy constructors be called while creating the vector. What am I doing wrong and how to force it to use move constructor or assignment?
#include <iostream>
#include <concepts>
#include <vector>
template<typename T>
requires std::is_arithmetic_v<T>
class Data {
private :
T mA = 0;
T mB = 0;
public:
Data(const T& data) : mA{data}{ // from single T
std::cout << "Constructed Data with: " << mA << ", " << mB << std::endl;
}
Data(const Data<T>& other) : mA{other.mA}, mB{other.mB} {
std::cout << "COPY Constructed Data with: " << mA << ", " << mB << std::endl;
}
Data(Data<T>&& other) : mA{other.mA}, mB{other.mB} {
std::cout << "MOVE Constructed Data with: " << mA << ", " << mB << std::endl;
}
Data(const std::initializer_list<T>& list) {
std::cout << "Constructed Data with list: ";
if(list.size() >= 2) {
mA = *list.begin();
mB = *(list.begin() + 1);
std:: cout << mA << ", " << mB << std::endl;
}
}
~Data() {
std::cout << "Destructed: " << mA << ", " << mB << std::endl;
}
const Data operator=(const Data& other) {
mA = other.mA;
mB = other.mB;
return *this;
}
Data operator=(Data&& other) {
mA = other.mA;
mB = other.mB;
return *this;
}
};
int main() {
std::cout << "** With initilizer_list **" << std::endl;
{
auto vec = std::vector<Data<int>>{{1,1}, {2,2}};
}
std::cout << "\n**With element**" << std::endl;
{
auto vec = std::vector<Data<int>>{1,2};
}
std::cout << "\n**With push**" << std::endl;
{
auto vec = std::vector<Data<int>>();
vec.push_back(1);
vec.push_back(2);
}
}
Output:
** With initilizer_list ** Constructed Data with list: 1, 1 Constructed Data with list: 2, 2 COPY Constructed Data with: 1, 1 COPY Constructed Data with: 2, 2 Destructed: 2, 2 Destructed: 1, 1 Destructed: 1, 1 Destructed: 2, 2 **With element** Constructed Data with: 1, 0 Constructed Data with: 2, 0 COPY Constructed Data with: 1, 0 COPY Constructed Data with: 2, 0 Destructed: 2, 0 Destructed: 1, 0 Destructed: 1, 0 Destructed: 2, 0 **With push** Constructed Data with: 1, 0 MOVE Constructed Data with: 1, 0 Destructed: 1, 0 Constructed Data with: 2, 0 MOVE Constructed Data with: 2, 0 COPY Constructed Data with: 1, 0 Destructed: 1, 0 Destructed: 2, 0 Destructed: 1, 0 Destructed: 2, 0
I expect the vectors to be filled in place
Both push_back and the initializer list constructor expect that the element is already constructed and passed through the parameter. Therefore the elements must first be constructed via conversion and then moved/copied into the vector's storage. If you don't want that, then use emplace_back instead, which takes constructor arguments for the element's constructor and creates the element in-place.
This doesn't resolve all cases of copy and moves, since the vector must move objects to new storage if the old one becomes too small to hold more elements. To avoid this completely, first call vec.reserve(...) where ... is at least as large as the maximum size that the vector will have.
The reason copy instead of move is used in case of reallocation is because you didn't mark your move constructor noexcept. std::vector prefers the copy constructor if the move constructor is not noexcept, because if a move constructor throws while moving the objects to new storage, then std::vector cannot guarantee that it will be able to roll-back to the previous state, as it would usually do.
Declaring copy/move constructor/assignment or a destructor in a class is always a bit risky. Firstly, because how easy it is to cause undefined behavior, see rule of three/five and secondly because it often results in worse results than the implicitly generated versions if not carefully written.
If you don't have a specific reason, e.g. because you manage a raw resource in the class, then don't declare any of these special members. (Only) Then the implicit ones will do the correct and most-likely best possible thing automatically ("rule of zero").