Linked list copying via recursion

[GreggoryLewis]

Hello,

So I am trying to implement a recursive method to copy a linked list. I have the general idea down (or at least I think), but could definitely use some guidance, because it isn't working.

Here is the iterative function I am trying to convert to a recursive call:

void LList<int>::copy(const LList<int>& right) {
    this->length = right.length;
    head = nullptr;

    if (this->length > 0) {
        head = new Node(right.head->value);
        Node* my = head;
        Node* copy = right.head;

        while (copy != nullptr) {
            Node* n = new Node(copy->value);
            my->next = n;
            my = n;
            copy = copy->next;
        }
    }
}

So I have modified the code of the copy method to then pass to the cRec (which is the recursive copy method) so that when copy is called, the private method "cRec" is then called. I am not simply changing "copy" because this linked list is inherited and requires a "copy" method.

Here is my new copy method passing to the cRec method:

void LList<int>::copy(const LList<int>& right) {
    this->length = right.length;
    head = nullptr;

    if (this->length > 0) {
        Node* my = head;
        Node* copy = right.head;

        cRec(my, copy);
    }
}

Here is my cRec method (which does not work)

void LList<int>::cRec(Node* my, Node* copy){
	if(copy == nullptr){
		return;
	}
	else{
		Node* n = new Node(copy->value);
		my->next = n;
		copy(my->next, copy->next);
                my = n;
	}
}

This isn't for a class. I am working on a persona programming project. Any assistance would be awesome! Thank you!

[ne555]

> which does not work
well, yeah, otherwise you wouldn't be here, ¿but what doesn't work?
we can't even compile the snips that you've posted, so you need to be a little more descriptive

void LList<int>::copy(const LList<int>& right) {
	this->length = right.length;
	head = nullptr;

	if (this->length > 0) {
		Node* my = head; // head is null, so my is mull too
		Node* copy = right.head;

		cRec(my, copy);
	}
}

void LList<int>::cRec(Node* my, Node* copy){
	if(copy == nullptr){
		return;
	}
	else{
		Node* n = new Node(copy->value);
		my->next = n; //my is nnull, you can't dereference it
		copy(my->next, copy->next); //¿where's your copy function that takes two arguments?
		my = n; //¿what's the point? this has no relevant effect
	}
}

PS: your original copy leaks memory

[jonnin]

this will work a lot better if your list had an insert function.
eg

void etc::copy(llist &in, llist tmp = head)  //you don't even need to fill tmp in when you call it!
{
    if(tmp)
     {
       in.insert(tmp);   
        copy(in, tmp->next); //the only place the second parameter will ever be seen is here
     }
   else
    return;
}

this is really 'append' or whatever depending on how 'insert' works. if you need to clear in before you copy into it, you should do that. here again it should be a method:
if(tmp == head) //cheap way to check if its the first time in the recursive calls
in.selfdestruct();
or whatever you name it.

recursion is a sharp doubled edged sword. Playing with it to learn it is fine, but only use it when the code is much easier to write with it. Copying a list is a while loop, no need to recurse:
while(tmp)
{
in.insert(tmp);
tmp = tmp->next;
}

doing this with recursion is harder to read, harder to debug, and usually earns you nothing for the effort.