CODEWITHSUNDEEP
cpointerslinked-listsingly-linked-list
Andrea
Andrea

Reputation: 15

Deleting from a List in C

Basically I have to delete a certain item from a linked list. This code works:

void delete_(Item client){
    link s=head,r;
    if(head->item==client){
        r=head;
        head=head->next;
        free(r);
    }
    else{
        while(s->next!=NULL){
            if(s->next->item==client){
                r=s->next;
                s->next=s->next->next;
                free(r);
            }
            else
                s=s->next;
        }
    }
}

Now I tried to reduce and compact the code using a for with 2 pointer but I can't figure out how to make it works. Here's the code:

void delete_(Item client){
    link x,r,p;
    for(x=head;x!=NULL;p=x,x=x->next){
        if(x->item==client){
            r=x;
            p->next=x->next;
            free(r);
        }
    }
}

Upvotes: 0

Views: 96

Answers (3)

M Oehm
M Oehm

Reputation: 29126

When you delete an item, you must know which pointer to update. That means you must know about the previous node in the list. Vlad's answer does this by keeping an extra previous variable, your first code does that by looking at the pointer pointed to by the current node. Both have to treat deletion from the head as special case.

Your second code tries to simplify the code, but you lose the special case of deletion at the head and also update the iterator link after deletion, which you shouldn't. (Your original code correctly places the update in an else clause.)

A method to get rid of the special case of deletion at the head is to introduce one level of indirection by iterationg via a pointer to node pointer. That pointer holds the address of the "previous" pointer – the list head or the next pointer of the previous node. The rest is more or less like your original code, expect that the current node now is at *l instead of at l->next:

void delete(Item client)
{
    link *l = &head;

    while (*l) {
        if ((*l)->client == client) {
            link r = *l;

            *l = (*l)->next;
            free(r);
        } else {
            l = &(*l)->next;
        }
    }
}

This code removes all items that match client; I reckon that is the desired behaviour. (The additional indirection also works for insertion.)

Upvotes: 0

Vlad from Moscow
Vlad from Moscow

Reputation: 310990

You can use for example the following appeoach

void delete( Item client )
{
    link current = head, previous = NULL;

    while ( current && current->item != client )
    {
        previous = current;
        current = current->next;
    }

    if ( current )
    {
        if ( !previous ) head = head->next;
        else previous->next = current->next;

        free( current );
    }
}

If you want to delete all nodes that have member item equal to client then indeed you can use a for loop.

For example

void delete( Item client )
{
    for( link current = head, previous = NULL; current != NULL; )
    {
        if ( current->item == client )
        {
            link tmp = current;

            if ( previous != NULL ) 
            {
               previous->next = current->next;
               current = current->next;
            }
            else
            {
                head = current->next;
                current = head;
            }

            free( tmp );
        }
        else
        {
            previous = current;
            current = current->next;
        }
    }
}

Upvotes: 1

Viet
Viet

Reputation: 583

There are two wrong points:

  • if the first element is the item need to be deleted, the previous of this item is not exist, and the code p->next = ... is not correct action. You should change the head of the list, it is right action.
  • when you delete current item(free(r)), so if you call x=x->next your program maybe crash. You must backup x->next before you delete. And the for loop of you need to change

Upvotes: 1

Related Questions