Search code examples
c++copy-constructordoubly-linked-listcircular-list

Circular Doubly Linked List Copy Constructor C++


I am trying to implement a copy constructor into my Circular Doubly Linked List, but I can not make it work. The files do copy, but not in the correct order. (NOTE: plate is defined as template<typename T>, and I will try to include the relevant functions only)

Class:

plate class CircularDoubleDirectedList : public ICircularDoubleDirectedList<T> {
private:
    plate class Node {
    public:
        T data;
        Node *next;
        Node *prev;
    };
    Node<T> *current;
    int nrOfElements;
    direction currentDirection;
public:
    CircularDoubleDirectedList() { nrOfElements = 0; currentDirection = FORWARD; current = nullptr; }
    virtual ~CircularDoubleDirectedList();
    CircularDoubleDirectedList(const CircularDoubleDirectedList<T>& origObj);
    CircularDoubleDirectedList& operator=(const CircularDoubleDirectedList<T>& origObj);
    void addAtCurrent(const T& element);
    T getElementAtCurrent() const;
    void removeAtCurrent();
    int size() const;
    void changeDirection();
    void moveCurrent();
    direction getCurrentDirection() const;
}; 

My try on the copy constructor:

plate CircularDoubleDirectedList<T>::CircularDoubleDirectedList(const CircularDoubleDirectedList<T>& origObj) {
    current = nullptr;
    nrOfElements = 0;
    Node<T> *tmp = origObj.current;
    currentDirection = origObj.currentDirection;

    while (nrOfElements < origObj.nrOfElements) {
        addAtCurrent(tmp->data);
        tmp = tmp->next;
    }
}

Adder:

plate void CircularDoubleDirectedList<T>::addAtCurrent(const T& element) {
    Node<T> *tmp = new Node<T>;
    tmp->data = element;
    tmp->next = nullptr;
    tmp->prev = nullptr;

    if (current == nullptr) {
        tmp->next = tmp;
        tmp->prev = tmp;
        }

    else if (nrOfElements == 1) {
        tmp->next = current;
        tmp->prev = current;
        current->next = tmp;
        current->prev = tmp;
    }

    else {

        if (currentDirection == FORWARD) {
            tmp->prev = current;
            tmp->next = current->next;
            current->next->prev = tmp;
            current->next = tmp;
        }

        else if (currentDirection == BACKWARD) {
            tmp->prev = current->prev;
            tmp->next = current;
            current->prev->next = tmp->prev;
            current->prev = tmp;
        }
    }

    nrOfElements += 1;
    current = tmp;
}

Thank you.


Solution

  • As I pointed out in the comment, the problem seems to be in the last function. So I try to write it from scratch in a more readable way:

    plate void CircularDoubleDirectedList<T>::addAtCurrent(const T& element) {
        Node<T> *tmp = new Node<T>;
        tmp->data = element;
    
        if (current == nullptr) {
            tmp->next = tmp;
            tmp->prev = tmp;
        } else {
            Node<T> * before, after;
            if (currentDirection == FORWARD) {
                before = current;
                after = current->next;
            } else {  // BACKWARD
                before = current->prev;
                after = current;
            }
    
            before->next = tmp;
            tmp->prev = before;
            after->prev = tmp;
            tmp->next = after;
        }
    
        nrOfElements += 1;
        current = tmp;
    }
    

    I see another problem in the copy constructor: if the currentDirection is Backward you are reading the elements from left to right, but adding the elements in a "afterward" fashion, with a consequent loss of the order. There are two solutions: you can respect the order during the scan, or set the currentDirection to FORWARD and then set it to the right value.

    plate CircularDoubleDirectedList<T>::CircularDoubleDirectedList(const CircularDoubleDirectedList<T>& origObj) {
        current = nullptr;
        nrOfElements = 0;
        currentDirection = FORWARD;  // set the scan direction temporarily
    
        for (Node<T> *tmp = origObj.current; nrOfElements < origObj.nrOfElements; tmp = tmp->next) {
            addAtCurrent(tmp->data);
        }
        if (nrOfElements > 0)
            current = current->next;  // align with the current of the copyed list
        currentDirection = origObj.currentDirection;  // set the right direction
    }
    

    please let me know if it solves your problem.