Doubly-linked List copy constructor (can't copy tail-to-head)

Question

So, I'm trying to implement a copy constructor that, upon instantiating an object, I can choose to copy a list from head-to-tail (normal) or tail-to-head (reverse). Now, when I use the copy constructor, it always copies head to tail, even if it enters the reverse condition.

Here is my header file:

#include <iostream>
#include "Student.h"
#include "Node.h"
using namespace std;

class List {

public:
    List(); // Default constructor
    List(const List&); // Copy constructor (2-in-1, see lab sheet)
    ~List(); // Destructor

    bool isEmpty(); // List is empty: TRUE or FALSE?
    int getNumNodes() {return numNodes;} // How many Nodes in List

    void append(Student *); // Append new Node to head or tail of List
    void insert(Student *); // Inserts new Node in the
                            // Appropriate location in List
    void deleteNode(string); //Search for and delete specific Node
    void displayAscending();// Display List HEAD to TAIL
    void displayDescending(); // Display List TAIL to HEAD

    // input Student::data into Student pointer.
    void input(Student*, string, string, string, string, string);

    Node *getHead() const {return head;} // ptr to head. 
    Node *getTail() const {return tail;} //ptr to tail.

private:
    void printer(Node *); //recursive function w/in displayDescending()
    Node *head;
    Node *tail;

    bool empty;
    bool forward; // forward = head-to-tail i.e. true
    int numNodes; // number of nodes in the list 
};

Here is my copy constructor.

List::List(List &list) { // Copy constructor
    head = NULL; // Head pointer set to NULL initially
    tail = NULL; // Tail pointer set to NULL initially

    empty = true;
    forward = true; // Copies head-to-tail by default.
    numNodes = 0;

    string flag; // Stores prompt value.
    cout << "Copy from head to tail? (y/n): ";
    cin >> flag; // prompt for user.
    if(flag == "n")
        forward = false;

    Node *curr = NULL; //initialize curr in function scope.

    if(flag == "n") {
        forward = false;
        curr = list.getTail(); // curr points to list tail.
        cout << "Copying in reverse order...\n" << endl;
    } else { // forward == true
        curr = list.getHead(); // curr points to list head.
        cout << "Copying from head-to-tail...\n" << endl;
    } // end if/else 

    while(curr) {
        string f = ( curr->getData()->getFirst()  );
        string m = ( curr->getData()->getMid()    );
        string l = ( curr->getData()->getLast()   );
        string s = ( curr->getData()->getSocial() );
        string a = ( curr->getData()->getAge()    );

        Node *nodePtr = NULL; // a node that's pointing 
                              // using it to point to creation of
                              // a new node
        Student *stuPtr = new Student; // creates a stud pointer on
                                       // heap of Student class in
                                       // order to store stud info 
        input(stuPtr,f,m,l,s,a); // input Student::data into stuPtr.      
        append(stuPtr); // append the node with stuPtr to head or tail
                        // of list.

        if(!forward)
            curr = curr->getPrev();
        else
            curr = curr->getNext();
    }  // end while*/
    cout << "Done copying!\n" << endl;
} // end copy constructor

Also, here's the append() function if you need to see how it appends to the list.

void List::append(Student *newStudent) {
    Node *newNode = new Node(newStudent); // new Node containing student arg.

    newNode->getData(); // get data of student arg.

    if(isEmpty()) {  // tail=NULL, no list.
        cout << "List is empty. Inserting first Node.\n" << endl;
        head = newNode;
        tail = newNode; // new Node becomes head & tail.
    } else { 
        if(forward) { // append to tail of list.
            tail->setNext(newNode); // NEXT ptr of tail points to newNode.
            newNode->setPrev(tail); // newNode's PREV points to former tail.
            tail = newNode; // newNode becomes the new tail.
        } else { // append to head of list.
            head->setPrev(newNode); // PREV ptr of head points to newNode.
            newNode->setNext(head); // newNode's NEXT points to former head.
            head = newNode; // newNode becomes the new head.
        } // end if/else
    } // end if/else
    numNodes++;
}

Answer 1

The reason you always seem to be copying from head to tail is that your code is unkempt and tries to do the same thing in more than one way at once. If you try to copy from tail to head, you write the new list from tail to head and you read the old list from tail to head. These two cancel each other out. Think about it.

The reason you get compiler errors if you try to make the argument const is that you're querying it with getTail() and getHead(), which you have not made const.

EDIT:

Let's go back to the design and think about how tail-to-head copying should work. There are basically two ways to do it, read head-tail and write tail-to-head:

|                |
v                v
A-B-C-D          A

  |            |
  v            v
A-B-C-D        B-A

    |        |
    v        v
A-B-C-D      C-B-A

      |    |
      v    v
A-B-C-D    D-C-B-A

or vise-versa:

      |    |
      v    v
A-B-C-D    D

    |        |
    v        v
A-B-C-D    D-C

  |            |
  v            v
A-B-C-D    D-C-B

|                |
v                v
A-B-C-D    D-C-B-A

Bur if we try to do both, they cancel out:

      |          |
      v          v
A-B-C-D          D

    |          |
    v          v
A-B-C-D        C-D

  |          |
  v          v
A-B-C-D      B-C-D

|          |
v          v
A-B-C-D    A-B-C-D

All we have to do is choose one. If we choose the first one, we change the copy ctor:

curr = list.getHead(); // curr points to list head.
cout << "Reading from head-to-tail...\n" << endl;

while(curr) {
  ...
  append(stuPtr); // append the node with stuPtr to head or tail of list.

  curr = curr->getNext();
}  // end while*/

And it works. If we choose the second, we leave the ctor alone and change append(...):

if(isEmpty()) {  // tail=NULL, no list.
  cout << "List is empty. Inserting first Node.\n" << endl;
  head = newNode;
  tail = newNode; // new Node becomes head & tail.
} else {
  tail->setNext(newNode); // NEXT ptr of tail points to newNode.
  newNode->setPrev(tail); // newNode's PREV points to former tail.
  tail = newNode; // newNode becomes the new tail.
} // end if/else

Generally speaking, the way to avoid problems like this is to start small and simple, add complexity a little at a time, test at every step, test new features in isolation first, and never add to code that doesn't work. The way to find non-obvious bugs is to take your code and simplify, removing complexity piecemeal until you reach the simplest version that still exhibits the buggy behavior-- or more likely the bug becomes obvious along the way.

Answer 2

By having a const argument, you're calling a non-const member function on const object. Non-const functions do not promise to not modify the object. You can declare these member functions as const to avoid this error:

Node *getHead() const

.. and so on

As for your first question, I don't know how to solve it (yet), I have a few recommendations:

• Use nullptr instead of NULL or 0
• Use list initialisation for your strings: string f { curr->getData()->getFirst() };

I suggest you debug your code line by line, using a good debugger tool, like Visual Studio; it will help you solve your problem in minutes. If you are still unsure, you can cut the amount of code to be tested, down to a few lines and post it.