1

So I have been coding in C++ for not so long and am struggling with a copy constructor and my program keeps on crashing when I use it.

Node:

template <class T>
class Node
{
public:
    Node(T data, Node<T> * n = 0)
    {   element = data;
        next = n;
        prev = n;}
    ~Node()
    {   next = 0;
        prev = 0;}
    T element;
    Node<T>* next;
    Node<T>* prev;

Function Code:

CircularList<T>::CircularList(const CircularList<T> & other)
{
    if(other.tail == NULL)
    {
        this->tail = NULL;
    }
    else
    {
        this->tail = other.tail;
        Node<T> * original = this->tail;
        Node<T> * temp = other.tail->next;
        while(temp != other.tail)
        {
            original->next = temp;
            temp = temp->next;
            original = original->next;
        }
        original->next = temp;

    }
    
}

(EDIT) Here is the specification for the CircularList Class. I think I just do not understand what steps to take to make it deep copy rather than shallow copy.

template<class T>
class CircularList;

template<class T>
ostream& operator<<(ostream&,CircularList<T>&);

template<class T>
class CircularList : public LinearStructure<T>
{
public:
    friend ostream& operator<< <T>(ostream&,CircularList<T>&);
    CircularList();
    CircularList(const CircularList<T>& other);
    CircularList<T>& operator=(const CircularList<T>& other);
    virtual CircularList<T>* clone();
    virtual ~CircularList();
    virtual void insert(int index, T element);
    virtual T remove(int index);
    virtual T get(int index) const;
    virtual bool isEmpty();
    virtual void clear();
    Node<T>* getLeader();

protected:
    ostream& print(ostream& os);


private:
    int size() const;
    Node<T>* tail;
};

So yeah when I test it with this (the list isn't empty):

CircularList<int> clist;
clist.insert(0,8);
CircularList<int> clist2(clist);

It crashes. Any help would be appreciated!!

Morgan Else
  • 11
  • 3
  • 2
    Your copy constructor does not copy. Both of the `CircularList` point to the same nodes because you are just copying pointers. – François Andrieux Nov 12 '20 at 19:38
  • Related (possibly dupe): [Deep copy vs Shallow Copy](https://stackoverflow.com/q/2657810/2602718) – scohe001 Nov 12 '20 at 19:40
  • You failed to unconditionally initialize `next` and `prev` in your copy constructor. When writing a constructor, regardless of whether it is the copy constructor, default constructor, or some other constructor, all the data members should wind up with known values. – PaulMcKenzie Nov 12 '20 at 19:43
  • Also, you should show your CircularList class. My previous comment applies if you have other data members in `CircularList` that will have their initializations not occur. That's why you should always use the member-initialization list, so that the list is visible to you and everyone else, showing that all members are initialized: `CircularList(const CircularList& other) : tail(nullptr) { .. }` – PaulMcKenzie Nov 12 '20 at 20:03
  • @PaulMcKenzie because the task is to create a singly linked circular list, I thought that using prev would make it doubly linked. I will edit the post to show the rest of the CircularList class – Morgan Else Nov 13 '20 at 06:00
  • You're not quite understanding the point of using the [member-initialization list](https://stackoverflow.com/questions/7665021/c-member-initialization-list) when constructing an object. The second thing is that a deep-copy means that once the copy construction is done, the copy is independent of the original, but is still a copy. By copying pointers as you're doing now, all you've really done is tie the objects together. You will immediately see this as soon as you attempt to destroy the original and the copy, as the destructor will be called twice on the same pointer value, causing errors. – PaulMcKenzie Nov 13 '20 at 14:12
  • That means that you have to allocate a *brand new* list, and copy the values to that list. If you have a "getFirst" and a "getNext" type of access to the elements, then the copy constructor can potentially be a simple loop that inserts all of the elements from the original into the copy. – PaulMcKenzie Nov 13 '20 at 14:14

0 Answers0