Greetings all DaniWeb people, long time no see.

I have a (hopefully) easy question. Let's jump into the code.
My test harness:

cout << "--------------------------------------------------------" << endl;
    cout << "Begin Copy Constructor Testing" << endl;
    cout << "--------------------------------------------------------" << endl << endl;
    dlSL = new DLSortedList();
    for (int i = 0; i <= 30; i += 2) dlSL->insert(i);
    dlSL->insert(11);
    cout << "Original list before copy:" << endl;
    dlSL->display();
    //clone the original list
    DLSortedList * newList = new DLSortedList(dlSL&);

My constructor for DLSortedList:

DLSortedList::DLSortedList() :
head(new DLNode::DLNode()),
tail(new DLNode::DLNode(head, 0, NULL)),
numItems(0),
cursor(head),
cursPosition(0) {
    head->item = INT_MIN;
    tail->item = INT_MAX;
    head->next = tail;
}

Copy constructor:

DLSortedList::DLSortedList(const DLSortedList& source) :
head(new DLNode::DLNode()),
tail(new DLNode::DLNode(head, 0, NULL)),
cursor(head),
cursPosition(0),
numItems(0) {
    head->item = INT_MIN;
    tail->item = INT_MAX;
    //is the source list empty?
    //if so, just point head at tail
    if (source.isEmpty())
        head->next = tail;
        //otherwise, we have to iterate through the old list and copy each node
    else {
        //setup first node
        DLNode * toInsert = new DLNode();
        toInsert->previous = head;
        toInsert->next = tail;
        tail->previous = toInsert;
        head->next = toInsert;
        toInsert->item = source.head->next->item;
        numItems += 1;
        cursPosition += 1;
        cursor = toInsert;
        //now copy the rest of the source
        DLNode * newPtr = toInsert;
        //newPtr points to last node in new list
        //sourcePtr points to nodes in original list
        //as long as the sourcePtr isn't pointing at the tail
        //we create a new DLNode, adjust pointers in the copy, and copy the item
        for (DLNode *sourcePtr = source.head->next->next;
                sourcePtr != tail;
                sourcePtr = sourcePtr->next) {
            newPtr->next = new DLNode();
            newPtr = newPtr->next;
            newPtr->item = sourcePtr->item;
            newPtr->next = tail;
            newPtr->previous = tail->previous;
            tail->previous = newPtr;
            numItems += 1;
            //should we move the cursor to the new item?
            if (cursPosition < source.cursPosition) {
                cursPosition += 1;
                cursor = newPtr;
            } //end if
        } //end for
    } //end else
} //end copy constructor

The class compiles, but the test will not.
I have tried to invoke the copy constructor a number of ways, but it is just not working.

DLSortedList * newList = new DLSortedList(dlSL&); 
//generates error: expected primary-expression before ‘)’ token
DLSortedList newList(dlSL&);
//generates error: expected primary-expression before ‘)’ token

I tried to invoke the copy constructor a few other ways but the errors generated there seemed to be worse. This is close, just need a bit of help to get over my hump.

Thanks all,
Vance

what is the & for in DLSortedList * newList = new DLSortedList(dlSL&) ? i think you might want to be doing DLSortedList * newList = new DLSortedList(&dlSL)

Here are some various guesses at getting the call right:

DLSortedList newList(&dlSL);

DLTest.cxx:64: error: no matching function for call to ‘DLSortedList::DLSortedList(DLSortedList**)’
DLSortedList.h:38: note: candidates are: DLSortedList::DLSortedList(const DLSortedList&)
DLSortedList.h:36: note: DLSortedList::DLSortedList()

DLSortedList * newList(&dlSL);

DLTest.cxx:64: error: cannot convert ‘DLSortedList**’ to ‘DLSortedList*’ in initialization
DLTest.cxx:66: error: request for member ‘display’ in ‘newList’, which is of non-class type ‘DLSortedList*’
DLTest.cxx:71: error: request for member ‘display’ in ‘newList’, which is of non-class type ‘DLSortedList*’
DLTest.cxx:73: error: request for member ‘insert’ in ‘newList’, which is of non-class type ‘DLSortedList*’
DLTest.cxx:74: error: request for member ‘display’ in ‘newList’, which is of non-class type ‘DLSortedList*’

Your suggestion of

DLSortedList * newList = new DLSortedList(&dlSL);

emits

DLTest.cxx:64: error: no matching function for call to ‘DLSortedList::DLSortedList(DLSortedList**)’
DLSortedList.h:38: note: candidates are: DLSortedList::DLSortedList(const DLSortedList&)
DLSortedList.h:36: note: DLSortedList::DLSortedList()

This of course led me to try

DLSortedList * newList = new DLSortedList(*dlSL);

which compiles and now the deep copy constructor is causing a SEGFAULT.

That's progress I guess...
I'll post the correct copy constructor when I get it working.

Thanks,
Vance

Without all the code to test this, we are just guessing. BUT: the for loop in the copy constructor looks horribly suspicious.

for (DLNode *sourcePtr = source.head->next->next;
	sourcePtr != tail;
	 sourcePtr = sourcePtr->next) 
{
  // ...
}

You see that tail is set to tail(new DLNode::DLNode(head, 0, NULL)) so despite the inserts it remains within the scope of the new list. Therefore,
sourcePtr which is within the realm of the old list cannot ever be equal to it.

This is just a guess. I don't feel I have enough of the code to evaluate it further. I hope this helps. [It is an easy one to put a test in for.]

Edited 5 Years Ago by StuXYZ: n/a

StuXYZ, you are exactly right.

I should have been testing for sourcePtr != source.tail.

Now it runs fine but I have a memleak somewhere... shouldn't be too tough to figure that out.

Thanks a million bro!

-Vance

This article has been dead for over six months. Start a new discussion instead.