It seems i made a mistake... but why this simple program doesn't work?


  • 0
    B

    Yes your code works well, would you please tell me why my code doesn't work?

    void deleteNode(ListNode* node) {
        while(node->next!=NULL){
            node->val = (node->next)->val;
            node = node->next;
        }
        delete node;
        node = NULL;
    }
    

    What i want to do is just copy the value in the next node to the current node. For a given node, if it's "next" isn't NULL, then set the "val" using the "val" in (node->next), then move the pointer to the next one. once the pointer hits the rear node, it's "next" should be NULL and then we quit the "while" and delete the corresponding node.

    But the reality is that this is a bad idea = =
    can you tell me why?


  • 2

    node is just your local variable. Setting node = NULL; at the end of your function has absolutely no effect whatsoever on the list. It does not make the next attribute of the previous node a null pointer.


  • 0
    B

    of course you're right, but i think the main problem of my function is not this one. Even i remove the last line, it doesn't work as before.
    I've read some AC codes, i fully understand the way they solve this problem——use two pointers to indicate the current node and the next one. The problem is that i think my function can do the same thing...
    There must be something wrong in the "while" loop..


  • 0

    "of course you're right, but i think the main problem of my function is not this one"

    It definitely is. Not making the next attribute of the previous node a null pointer is the problem.

    You can do it inside the loop or after it, up to you. You just need to actually do it somewhere.

    "Even i remove the last line, it doesn't work as before."

    What do you mean with "doesn't work as before"? It worked before? Or it worked differently?

    Removing the last line doesn't have any noticeable effect. That line has absolutely no effect whatsoever other than changing the variable's value right before it's thrown away anyway. The compiler might even ignore that line if optimization is turned on.


  • 1
    Y

    I think your idea is correct. I guess the problem is that the second last node (in the original list) points to the last node which you finally deleted. Since you didn't set the next pointer of the second last node to NULL, you didn't have an end node (node->next==NULL) in your new list.


  • 0
    B

    really sorry for my limited english. Thank you a lot, i'll try to fix the problem, thank you~


  • 0
    B

    Thank you! your advice is clear and useful. I realized my problem after reading your answer, then I fixed the problem and get AC this time!

    void deleteNode(ListNode* node) {
        while(node->next!=NULL){
            node->val = (node->next)->val;
            if(node->next->next==NULL){
                ListNode * last = node->next;
                node->next = NULL;
                node = last;
            }
            else
                node = node->next;
        }
        
        delete node;

Log in to reply
 

Looks like your connection to LeetCode Discuss was lost, please wait while we try to reconnect.