1

I am trying to understand the following 2 versions of implementation of assignment operator, which is used to assign the other dynamic array to the the instance (class DoubleVector) it was called with.

version 1.

    DoubleVector& DoubleVector::operator= (DoubleVector other)
    {
        swap(vector_, other.vector_);
        swap(size_, other.size_);

        return *this;
    }

version 2.

    DoubleVector& DoubleVector::operator= (const DoubleVector& other)
    {
        double* newVector = new double[other.size_];   // (Try to) allocate new memory

        for (int i = 0; i < other.size_; i++)
        {
            newVector[i] = other.vector_[i];
        }

        delete[] vector_;   // After allocation succeeded we can delete the old array

        size_ = other.size_;
        vector_ = newVector;

        return *this;
    }

My questions are:

  1. For the version 1, is there any case that may be missed (e.g. other.size_ = 0)?
  2. For the version 2, why we need to delete the old array (delete[] vector_;) after the allocation succeeded? Is it necessary?

  3. Furthermore, for the version 2, can I just directly assign other to the instance that "=" is called with? e.g.

    DoubleVector& DoubleVector::operator= (const DoubleVector& other)
{
   for (int i = 0; i < other.size_; i++)
   {
       vector_[i] = other.vector_[i];
   }

   size_ = other.size_;


   return *this;
}
livemyaerodream
  • 890
  • 6
  • 19
  • (3) Have a think about the length of `vector_` ? – Richard Critten Dec 09 '19 at 18:04
  • (1) is a copy/swap idiom implementation, relying entirely on the proficiency of your copy-ctor. (2) is manual, and given that implementation, absolutely no better (and in fact worse) than (1). Your proposed (3), think about what goes south when `this->size_ < other.size_`, which your proposal never checks, is in fact true. – WhozCraig Dec 09 '19 at 18:10
  • @WhozCraig For (1), why this copy/swap version assignment operator is related to the copy constructor? Copy constructor is called only when the instance is not already constructed, right? So in this case, which the instance is already constructed, this will not call the copy constructor. – livemyaerodream Dec 10 '19 at 16:26
  • @Hua-MingHuang Notice the `DoubleVector other` argument and how it differs in (1) from all the others? That's intentional. [See this answer](https://stackoverflow.com/a/3279550/1322972). – WhozCraig Dec 10 '19 at 19:05

2 Answers2

1

Notice that the array passed as a parameter to the two versions of copy operator is different.

In first case there is a DoubleVector value parameter passed by value (copy of passed value is created with copy constructor or copy assignment operator, in this case listed below). Since function operates with copy of data copy is replacement with swap due to efficiency reasons. All corner cases (like other.size == 0) will be processed correctly.

In the second case there is a const DoubleVector & value parameter passed by const reference. No copying of data is performed and to guarantee that external data will not be modified the reference is const (generally it's a good practice to use const qualifiers where applicable). In this case we manually allocate memory for future array (since currently allocated array, if any, may differ in size). realloc may also be used for that reason. Further internal pointer to array is set to newly allocated data: vector_ = newVector;. Before that assignment we must return previously allocated memory by calling delete[] vector_;. Otherwise there will be a memory leak. Consider 10^3 calls to this operator with array of 10^6 doubles.

The second method has one issue. There is no check on self-assignment:

DoubleVector& DoubleVector::operator= (const DoubleVector& other)
{
    if (this == &other)
        return;
    ...
}

Copying is a core concept of OOP. There are different solutions common in use: copy on write, reference copy, copy-swap idiom (mentioned in comments) and others. Additionally modern C++ introduces move concept.

Hope it helps.

f4f
  • 891
  • 5
  • 13
0
  1. Zero will be handled properly. There is nothing wrong with having an empty array. The behavior will result in "vector_" being an empty array.
  2. You have to delete the old "vector_" because a new one is being created and assigned to "vector_". If you did not delete it, then it would be a memory leak.
  3. You don't know the usage of "other" outside of this operator, so you should not do that assignment. The calling function could delete "other" out from under this instance (or visa-versa) and then you would have a crash/error to find/fix.
Topher
  • 451
  • 2
  • 8