Assuming I have assignment operator, is my copy constructor being this = obj okay?

Question

Essentially, my question is as follows: assuming I've created an assignment operator for a class, is it against convention or frowned upon to have my copy constructor just be this = item?

Lets say I'm creating a templated class with only the following data:

private:
    int       _size;
    ItemType* _array;

If my assignment operator is as follows:

template<class ItemType>
void obj<ItemType>::operator = (const obj & copyThis){
    _size = copyThis.getSize(); 
    _array = new ItemType[_size];
    for (int i = 0; i < _size; i++){
        //assuming getItemAt is a function that returns whatever is in given location of object's _array
        _array[i] = copyThis.getItemAt(i);
    }
}

Then would it be against convention/looked down upon/considered incorrect if my copy constructor was simply as follows?

template<class ItemType>
obj<ItemType>::obj(const obj & copyThis){
    this = copyThis;
}

Answer 1

It is generally safe to call operator= in the copy constructor (as long as the operator= does not try to use the copy constructor as part of its logic).

However, your operator= is implemented wrong to begin with. It leaks memory, does not handle this being assigned to itself, and does not return a reference to this.

Try this instead:

template<class ItemType>
obj<ItemType>::obj(const obj & copyThis)
    : _size(0), _array(0)
{
    *this = copyThis;
}

template<class ItemType>
obj<ItemType>& obj<ItemType>::operator=(const obj<ItemType> &copyThis)
{
    if (this != &copyThis)
    {
        int newSize = copyThis.getSize(); 
        ItemType *newArray = new ItemType[newSize];

        // consider using std::copy() instead:
        //
        // std::copy(copyThis._array, copyThis._array + newSize, newArray);
        //
        for (int i = 0; i < newSize; ++i) {
            newArray[i] = copyThis.getItemAt(i);
        }

        delete[] _array;
        _array = newArray;
        _size = newSize; 
    }

    return *this;
}

That being said, it is generally better to implement operator= using the copy constructor, not the other way around:

template<class ItemType>
obj<ItemType>::obj(const obj & copyThis)
    : _size(copyThis.getSize()), _array(new ItemType[_size])
{
    for (int i = 0; i < _size; ++i){
        _array[i] = copyThis.getItemAt(i);
    }

    // or:
    // std::copy(copyThis._array, copyThis._array + _size, _array);
}

template<class ItemType>
obj<ItemType>& obj<ItemType>::operator=(const obj<ItemType> &copyThis)
{
    if (this != &copyThis)
    {
        obj<ItemType> tmp(copyThis); 
        std::swap(_array, tmp._array);
        std::swap(_size, tmp._size);
    }

    return *this;
}

Which can be cleaned up a little if you add a swap method:

template<class ItemType>
obj<ItemType>::obj(const obj & copyThis)
    : _size(copyThis.getSize()), _array(new ItemType[_size])
{
    for (int i = 0; i < _size; ++i){
        _array[i] = copyThis.getItemAt(i);
    }
}

template<class ItemType>
void obj<ItemType>::swap(obj<ItemType> &swapThis)
{
    std::swap(_array, swapThis._array);
    std::swap(_size, swapThis._size);
}

template<class ItemType>
obj<ItemType>& obj<ItemType>::operator=(const obj<ItemType> &copyThis)
{
    if (this != &copyThis) {
        obj<ItemType>(copyThis).swap(*this); 
    }

    return *this;
}

That being said, if you replace your manual array with a std::vector, then you don't need to manually implement a copy constructor or a copy assignment operator at all, the compiler-generated default ones will suffice (since std::vector already implements copy semantics).