linked list delete function

Question

The following code snippet is not working right.

void deleteNode(list **start, int pos) {
    int currentPosition=0;
    list *currentNode;
    list *nodToDelete;

    currentNode = *start;
    if (currentNode == NULL) {
        printf("Empty List\n");
    } else if (pos == 0 ) {
        nodToDelete = *start;
        *start = nodToDelete->next;
        free(nodToDelete);
    } else {
        while (currentNode->next != NULL) {
            if (currentPosition >= pos -1) {
                break;
            }
            currentPosition++;
            currentNode = currentNode->next;
        }
        if (currentPosition < pos -1 || currentNode->next == NULL) {
            printf("No node at given position exists\n");
        } else {
            nodToDelete = currentNode->next;
            currentNode = nodToDelete->next;
            free(nodToDelete);
            nodToDelete = NULL;
        }
    }
}

void displayList(list *node) {
    if (node == NULL) {
        printf("Empty List");
    }

    while (node != NULL) {
        printf("%d\t", node->data);
        node = node->next;
    }
    printf("\n");
}

int main()
{
    list *start, *node;
    start = NULL;

    insertNode(&start, 2);
    insertNode(&start, 3);
    insertNode(&start, 4);
    insertNode(&start, 1);
    insertNode(&start, 5);

    deleteNode(&start, 3);

    displayList(start);
}

When executed the output is

Before Deletion 2 3 4 1 5
After Deletion 2 3 4 0 5

It is supposed to delete 1 but it is inserting 0 at its place.

Your code is incomplete; please show the definition of list. — zwol
– zwol, Commented Jun 18, 2011 at 3:58
Shouldn't currentNode = nodToDelete->next be currentNode->next = nodToDelete->next; — Ben
– Ben, Commented Jun 18, 2011 at 4:09

Sai · Accepted Answer · 2011-06-18 04:11:09Z

2

Here is something that might work -- Replace

currentNode = nodToDelete->next;

with

currentNode->next = nodToDelete->next;

You basically need the node before the nodetodelete to have its next to point to the node that nodetodelete used to point to

answered Jun 18, 2011 at 4:11

Sai

3,9871 gold badge27 silver badges29 bronze badges

Sign up to request clarification or add additional context in comments.

Comments

rakslice · Accepted Answer · 2011-06-18 04:08:35Z

1

Once you've found the node you want to take out of the list, you need to actually take it out. =)

...

nodToDelete = currentNode->next;
currentNode->next = nodToDelete->next;
free(nodToDelete);

...

answered Jun 18, 2011 at 4:08

rakslice

9,0354 gold badges58 silver badges63 bronze badges

1 Comment

user804195 Over a year ago

thanks for the solution so ultimately it turned out to be a 0 level bug, which i could not figure out

jmoreno · Accepted Answer · 2011-06-18 06:04:27Z

Besides the problem with currentNode->next = nodToDelete->next; and negative positions you are mixing your ui and your logic. As much as possible you should separate the two.

Sending something to the ui is a way of reporting progress; whether the ui is a command line, a browser or a speaker. Within deleteNode, an empty list or a position that is out of bounds, is not progress. Sequentially both are the same as success - you are done. If you want failure to be to be reported, that should be done where it can lead to a separate sequence...i.e the caller. Also, by mixing in ui, you introduce an unnecessary dependency and failure (what if there's a bug in printf, YOUR function will crash when it doesn't doesn't have to). If you're function returns a defined result, the caller can decide if/how to report that result, including success (your function currently doesn't do so, and the caller has no way telling the difference between sucess or failure).

Collectives™ on Stack Overflow

linked list delete function

3 Answers 3

Comments

1 Comment

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

Comments

1 Comment

Comments

Your Answer

Sign up or log in

Post as a guest

Related