Trying to figure out a Copy Constructor for the course class

Question

This is the course class in question.

class Course {
    //** You may not change the declarations in this private: area.
    CourseName name;           ///< Name of the course
    int numberOfPrerequisites; ///< How many prereqs are currently in the array?
    int maxPrerequisites;      ///< How many prereqs will fit into the array?
    CourseName * prereqs;  

Here is what I have as of now. Note that this copy constructor causes some of my unit tests to fail. I cannot change the unit test but instead must fix the copy constructor:

Course::Course(const Course& other)
    : name(other.name), numberOfPrerequisites(other.numberOfPrerequisites),
      maxPrerequisites(other.maxPrerequisites), prereqs(new CourseName[other.maxPrerequisites])
{
    // Copy all prerequisites
    for (int i = 0; i < numberOfPrerequisites; ++i)
    {
        prereqs[i] = other.prereqs[i];
    }
}

The unit test that are failing say that once a new course is initialized using another course, when you modify the newly create course it is also changing the original course. Simply put they are the same after modifications when they should be different.

These are the operator overloads that I have tried thus far.

bool Course::operator==(const Course& other) const
{
    return name == other.name;
}

Course& Course::operator=(const Course& other)
{
    if (this == &other)
        return *this;

    name = other.name;
    numberOfPrerequisites = other.numberOfPrerequisites;
    maxPrerequisites = other.maxPrerequisites;

    delete[] prereqs;

    prereqs = new CourseName[other.maxPrerequisites];

    // Copy the prerequisites from the source object to the new object
    for (int i = 0; i < numberOfPrerequisites; ++i)
    {
        prereqs[i] = other.prereqs[i];
    }

    return *this;
}

Answer 1

This seems to be a homework assignment of some sort, so I do not want to give away too much in this answer.

That said, the comments have identified these things:

1. The copy-ctor appears to be okay.
2. So does the copy-assignment operator=.
3. The dtor is missing.
4. The equality operator== is deficient.

The missing dtor will cause memory leaks. Its absence, however, should not cause the copy-ctor or copy-assignment operator= to fail.

The unit test that are failing say that once a new course is initialized using another course, when you modify the newly create course it is also changing the original course.

If the copy-ctor is doing its job correctly–which seems to be the case–then the only thing that could be going wrong is the copying of CourseName.

At first, I assumed it had a straightforward definition such as this:

struct CourseName
{
    std::string name;
    bool operator== (CourseName const&) const = default;
};

But if that were true, then there would be no error in the copy-ctor.

That is when it occurred to me that this is a lesson about deep-copy and deep-compare. Did the instructor implement CourseName so that it, too, would require the "Rule of Three?"

struct CourseName
{
    char const* name_{ nullptr };
public:
    CourseName(char const* name);

    // Rule of Three
    ~CourseName();
    CourseName(CourseName const& other);
    CourseName& operator=(CourseName const& other);

    // Other member functions and operators
    bool operator== (CourseName const& other) const;
};

If so, then the compiler-supplied, default copy-assignment operator= for CourseName would not do the right thing. Neither would an equality operator== that did member-wise comparison.

Your problem, therefore, may be that you need to work on CourseName a bit more.

If, not, then we will need to see a Minimal, Reproducible Example in order to analyze further. That should be a complete program, including function main. An MRE excludes unnecessary detail, but otherwise reproduces the error.

What is the "Strong Guarantee?"

The so-called "strong guarantee" is a promise that when an operator such as operator= or operator>> fails, the original object is left untouched. It's all-or-nothing: no partial updates are allowed.

The operator= given in the original question does not provide the strong guarantee. It is possible that the call to operator new[] could fail, throwing std::bad_alloc. When that happens, you will not be able to recover the original object, because you will have already deleted it.

In order to provide the strong guarantee, you need to assign the result of operator new[] to to a temporary variable, and only update the object after that pointer has been safely received.

This is your operator=, with only the few changes needed to provide the strong guarantee.

Course& Course::operator=(const Course& other)
{
    if (this == &other)
        return *this;

    // If the call to operator new[] is going to fail, 
    // we need that to happen before we start deleting 
    // the original object. That is why we do this first.
    auto temp = new CourseName[other.maxPrerequisites];

    // Operator new[] did not throw std::bad_alloc.
    // Now it is safe to start overwriting things.
    name = other.name;
    numberOfPrerequisites = other.numberOfPrerequisites;
    maxPrerequisites = other.maxPrerequisites;

    delete[] prereqs;

    // Assign the temporary pointer created above.
    prereqs = temp;

    // Copy the prerequisites from the source object to the new object
    for (int i = 0; i < numberOfPrerequisites; ++i)
    {
        prereqs[i] = other.prereqs[i];
    }

    return *this;
}

As identified in the comments, the copy-and-swap idiom is an alternative way to implement operator=. Copy-and-swap has the benefit of simplicity, and, in addition, provides the strong guarantee.