Copy Constructors, Assignment Operator C++

Question

I am very new to OOP and am still trying to comprehend all the concepts of the constructors. I have a class with some data and I have to make a Copy Constructor and Assignment Operator, however, since this is the first time I am doing something like this, I am not sure if what I have written makes sense. So, I am asking if what I have written are valid Copy Constructor and Assignment Operator. The class is saved in a file called BKS.h Thank you! Here is the class :

#include <iostream>
#include <vector>
#include <cassert>
#include <algorithm>

using namespace std;

template <class T>
class BKS final
{
public:
    struct Item
    {
        T identifier;
        int weight;
        int benefit;
    };

    BKS() {}
    BKS(const BKS<T> &copy);
    BKS(const vector<Item> &items) : itemlist_{items} {}
    BKS(const vector<pair<int, int>> &weight_benefit_list);
    BKS<T> &operator=(const BKS<T> &copy);
    // some methods ....

private:
    vector<Item> itemlist_;
    vector<int> current_selection_;
    int current_capacity_ {0};
    int maximal_benefit_ {0};

};

Copy Constructor and Assigment Operator :

#include "bks.h"

template <class T>
BKS<T>::BKS(const BKS<T> &copy)                 // copy constructor 
{   
    std::vector<Item> itemlist_ = copy.itemlist_;
    std::vector<int> current_selection_ = copy.current_selection_;
    int current_capacity_ = copy.current_capacity_;
    int maximal_benefit_ = copy.maximal_benefit_;  
}

template <class T>
BKS<T> &BKS<T>::operator=(const BKS<T> &copy)
{
    if (&copy != this)
    { // check for self-assignment
        this->itemlist_ = copy.itemlist_;
        this->current_selection_ = copy.current_selection_;
        this->current_capacity_ = copy.current_capacity_;
        this->maximal_benefit_ = copy.maximal_benefit_;
    }
    return *this;
}

Also any general recommendations concerning constructors are welcome :)

You don't need a copy-ctor here, your class doesn't managa any data. The default generated one will be enough. But in general you are doing the right thing. You simplify the code by moving the actual assignment to a function that is then called by copy-ctor and `operator=` (less code duplication). — Lukas-T, Jul 06 '20 at 11:36
Well, it is my first time doing something similar, and I was wondering if there are some semantik mistakes, since I can't say that I really know how constructors must be coded. I just know what they do, but don't know how to write them. — Nikola, Jul 06 '20 at 11:36
You can find some examples of copy and move-ctors here: https://stackoverflow.com/questions/4172722/what-is-the-rule-of-three and something about simplifying things even more here: https://stackoverflow.com/questions/3279543/what-is-the-copy-and-swap-idiom?rq=1 Hope that helps to clarify things. — Lukas-T, Jul 06 '20 at 11:40
Your copy constructor is wrong. You should not declare any variables in it. — molbdnilo, Jul 06 '20 at 11:40
Aside from everyone else's comments, you have put the definition to a template class in a source file -- which will give you a bad time with the linker later. Templates usually should be defined all in one file so that the definition is visible during instantiation — Human-Compiler, Jul 06 '20 at 11:43
And after you read the links churill posted, you can look up the Rule Of Zero to understand the note that except because your instructor says to, the class shouldn't declare its copy constructor or assignment at all. — aschepler, Jul 06 '20 at 11:45

score 0 · Accepted Answer · answered Jul 06 '20 at 12:36

If your instructor insists you must declare special members, but gives no guidance on how, then the best way is:

template <class T>
class BKS final
{
public:
    ~BKS() = default;
    BKS(const BKS &) = default;
    BKS& operator=(const BKS &) = default;
    BKS(BKS &&) = default;
    BKS& operator=(BKS &&) = default;

    /* other members... */
};

If your instructor doesn't require you to declare them, but only requires they exist, the best way is

template <class T>
class BKS final
{
public:
    /* other members... */
};

score 0 · Answer 2 · answered Jul 06 '20 at 12:53

When you create a new class without declaring a copy constructor, a copy assignment operator and a destructor, compilers will declare their own versions of a copy constructor, a copy assignment operator and a destructor. Furthermore, if you don't declare your constructor, the compilers will implicitly declare one for you.

All these default compiler generated functions are all public and inline. Notice that the implictly-declared destructor is non-virtual.

So what will the default constructor, default copy constructor, default copy assignment operator and default destructor do?

Default constructor (either implicitly-declared or user-defined): calls the default constructors of the bases and of the non-static members of the class;
Default copy constructor(implicitly-declared) and default copy assignment operator(implicitly-declared): copy each non-static data member of the source object to the target object;
Default destructor(either implicitly-declared or user-defined): calls the destructors of the base class and members of the derived class.

Here in your cases, your non-static data members will be copied by default copy constructor and default copy assignment operator. There's no need defining your own copy constructor and copy assignment operator unless you want to add specific behaviors for your copy constructor and copy assignment operator.

References:

https://en.cppreference.com/w/cpp/language/default_constructor

https://www.ibm.com/support/knowledgecenter/SSLTBW_2.3.0/com.ibm.zos.v2r3.cbclx01/cplr380.htm

Book: "Effective C++" by Scott Meyers, Chapter 2

Copy Constructors, Assignment Operator C++

2 Answers2