Revisions to C function to find and delete a node from a singly linked list

Commonmark migration

Source Link

edited Jun 10, 2020 at 13:24

1

###General Comments:

General Comments:

Why do your list handling functions not take a list as a parameter?
As a result your application can only have one list.

Comments on Delete:

###Comments on Delete: YouYou are leaking the list item when you delete it.

Since the create_item() is calling malloc() I would expect the delete_item() to call free().

I would split the delete_item() into two parts. The first part that deals with the head as a special case and the second part that deals with removing elements from the list (and free()ing them).

void delete_item(struct node** list, int x)
{
    if ((list == NULL) || ((*list) == NULL))
    {
        printf("not found\n");
        return;
    }
    
    (*list) = delete_item_from_list(*list);
}

struct node* delete_item_from_list(struct node* head)
{
    struct node* iter = head;
    struct node* last = NULL;

    while (iter != NULL)
    {
        if (iter->x == x)
        {
            break;
        }

        last = iter;
        iter = iter->next;
    }

    if (iter == NULL)
    {
        printf("not found\n");
    }
    else if (last == NULL)
    {
        printf("found in first element: %i\n", x);
        head = iter->next;
    }
    else
    {
        printf("deleting element: %i\n", x);
        last->next = iter->next;
    }
   
    free(iter);
    return head;
}

###General Comments:

Why do your list handling functions not take a list as a parameter?
As a result your application can only have one list.

###Comments on Delete: You are leaking the list item when you delete it.

Since the create_item() is calling malloc() I would expect the delete_item() to call free().

I would split the delete_item() into two parts. The first part that deals with the head as a special case and the second part that deals with removing elements from the list (and free()ing them).

void delete_item(struct node** list, int x)
{
    if ((list == NULL) || ((*list) == NULL))
    {
        printf("not found\n");
        return;
    }
    
    (*list) = delete_item_from_list(*list);
}

struct node* delete_item_from_list(struct node* head)
{
    struct node* iter = head;
    struct node* last = NULL;

    while (iter != NULL)
    {
        if (iter->x == x)
        {
            break;
        }

        last = iter;
        iter = iter->next;
    }

    if (iter == NULL)
    {
        printf("not found\n");
    }
    else if (last == NULL)
    {
        printf("found in first element: %i\n", x);
        head = iter->next;
    }
    else
    {
        printf("deleting element: %i\n", x);
        last->next = iter->next;
    }
   
    free(iter);
    return head;
}

General Comments:

Why do your list handling functions not take a list as a parameter?
As a result your application can only have one list.

Comments on Delete:

You are leaking the list item when you delete it.

Since the create_item() is calling malloc() I would expect the delete_item() to call free().

I would split the delete_item() into two parts. The first part that deals with the head as a special case and the second part that deals with removing elements from the list (and free()ing them).

void delete_item(struct node** list, int x)
{
    if ((list == NULL) || ((*list) == NULL))
    {
        printf("not found\n");
        return;
    }
    
    (*list) = delete_item_from_list(*list);
}

struct node* delete_item_from_list(struct node* head)
{
    struct node* iter = head;
    struct node* last = NULL;

    while (iter != NULL)
    {
        if (iter->x == x)
        {
            break;
        }

        last = iter;
        iter = iter->next;
    }

    if (iter == NULL)
    {
        printf("not found\n");
    }
    else if (last == NULL)
    {
        printf("found in first element: %i\n", x);
        head = iter->next;
    }
    else
    {
        printf("deleting element: %i\n", x);
        last->next = iter->next;
    }
   
    free(iter);
    return head;
}

formatting code, spellcheck

Source Link

edited Feb 22, 2018 at 17:20

Snowbody

8.7k
25
50

###General Comments:

Why do your list handling functions not take a list as a parameter?
As a result your application can only have one list.

###Comments on Delete: You are leaking the list item when you delete it.

Since the create_item()create_item() is calling mallocmalloc() I would expect the delete_item()delete_item() to call freefree().

I would split the delete_item()delete_item() into two parts. The first part that deals with the head as a special case and the second part that deals with removing elemntselements from the list (and free()free()ing them).

void delete_item(struct node** list, int x)
{
    if ((list == NULL) || ((*list) == NULL))
    {
        printf("not found\n");
        return;
    }
    
    (*list) = delete_item_from_list(*list);
}

struct node* delete_item_from_list(struct node* head)
{
    struct node* iter = head;
    struct node* last = NULL;

    while (iter != NULL)
    {
        if (iter->x == x)
        {
            break;
        }

        last = iter;
        iter = iter->next;
    }

    if (iter == NULL)
    {
        printf("not found\n");
    }
    else if (last == NULL)
    {
        printf("found in first element: %i\n", x);
        head = iter->next;
    }
    else
    {
        printf("deleting element: %i\n", x);
        last->next = iter->next;
    }
   
    free(iter);
    return head;
}

###General Comments:

Why do your list handling functions not take a list as a parameter?
As a result your application can only have one list.

###Comments on Delete: You are leaking the list item when you delete it.

Since the create_item() is calling malloc I would expect the delete_item() to call free.

I would split the delete_item() into two parts. The first part that deals with the head as a special case and the second part that deals with removing elemnts from the list (and free()ing them).

void delete_item(struct node** list, int x)
{
    if ((list == NULL) || ((*list) == NULL))
    {
        printf("not found\n");
        return;
    }
    
    (*list) = delete_item_from_list(*list);
}

struct node* delete_item_from_list(struct node* head)
{
    struct node* iter = head;
    struct node* last = NULL;

    while (iter != NULL)
    {
        if (iter->x == x)
        {
            break;
        }

        last = iter;
        iter = iter->next;
    }

    if (iter == NULL)
    {
        printf("not found\n");
    }
    else if (last == NULL)
    {
        printf("found in first element: %i\n", x);
        head = iter->next;
    }
    else
    {
        printf("deleting element: %i\n", x);
        last->next = iter->next;
    }
   
    free(iter);
    return head;
}

###General Comments:

Why do your list handling functions not take a list as a parameter?
As a result your application can only have one list.

###Comments on Delete: You are leaking the list item when you delete it.

Since the create_item() is calling malloc() I would expect the delete_item() to call free().

I would split the delete_item() into two parts. The first part that deals with the head as a special case and the second part that deals with removing elements from the list (and free()ing them).

void delete_item(struct node** list, int x)
{
    if ((list == NULL) || ((*list) == NULL))
    {
        printf("not found\n");
        return;
    }
    
    (*list) = delete_item_from_list(*list);
}

struct node* delete_item_from_list(struct node* head)
{
    struct node* iter = head;
    struct node* last = NULL;

    while (iter != NULL)
    {
        if (iter->x == x)
        {
            break;
        }

        last = iter;
        iter = iter->next;
    }

    if (iter == NULL)
    {
        printf("not found\n");
    }
    else if (last == NULL)
    {
        printf("found in first element: %i\n", x);
        head = iter->next;
    }
    else
    {
        printf("deleting element: %i\n", x);
        last->next = iter->next;
    }
   
    free(iter);
    return head;
}

added 8 characters in body

Source Link

edited Feb 1, 2011 at 1:56

Loki Astari

97.7k
5
126
341

###General Comments:

Why do your list handling functions not take a list as a parameter?
As a result your application can only have one list.

###Comments on Delete: You are leaking the list item when you delete it.

Since the create_item() is calling malloc I would expect the delete_item() to call free.

I would split the delete_item() into two parts. The first part that deals with the head as a special case and the second part that deals with removing elemnts from the list (and free()ing them).

void delete_item(struct node** list, int x)
{
    if ((list == NULL) || ((*list) == NULL))
    {
        printf("not found\n");
        return;
    }
    
    (*list) = delete_item_from_list(*list);
}

struct node* delete_item_from_list(struct node* head)
{
    struct node* iter = head;
    struct node* last = NULL;

    while (iter != NULL)
    {
        if (iter->x == x)
        {
            break;
        }

        last = iter;
        iter = iter->next;
    }

    if (iter == NULL)
    {
        printf("not found\n");
    }
    else if (last == NULL)
    {
        printf("found in first element: %i\n", x);
        head = iter;iter->next;
    }
    else
    {
        printf("deleting element: %i\n", x);
        last.next->next = iter.next;->next;
    }
   
    free(iter);
    return head;
}

###General Comments:

Why do your list handling functions not take a list as a parameter?
As a result your application can only have one list.

###Comments on Delete: You are leaking the list item when you delete it.

Since the create_item() is calling malloc I would expect the delete_item() to call free.

I would split the delete_item() into two parts. The first part that deals with the head as a special case and the second part that deals with removing elemnts from the list (and free()ing them).

void delete_item(struct node** list, int x)
{
    if ((list == NULL) || ((*list) == NULL))
    {
        printf("not found\n");
        return;
    }
    
    (*list) = delete_item_from_list(*list);
}

struct node* delete_item_from_list(struct node* head)
{
    struct node* iter = head;
    struct node* last = NULL;

    while (iter != NULL)
    {
        if (iter->x == x)
        {
            break;
        }

        last = iter;
        iter = iter->next;
    }

    if (iter == NULL)
    {
        printf("not found\n");
    }
    else if (last == NULL)
    {
        printf("found in first element: %i\n", x);
        head = iter;
    }
    else
    {
        printf("deleting element: %i\n", x);
        last.next = iter.next;
    }
   
    free(iter);
    return head;
}

###General Comments:

Why do your list handling functions not take a list as a parameter?
As a result your application can only have one list.

###Comments on Delete: You are leaking the list item when you delete it.

Since the create_item() is calling malloc I would expect the delete_item() to call free.

I would split the delete_item() into two parts. The first part that deals with the head as a special case and the second part that deals with removing elemnts from the list (and free()ing them).

void delete_item(struct node** list, int x)
{
    if ((list == NULL) || ((*list) == NULL))
    {
        printf("not found\n");
        return;
    }
    
    (*list) = delete_item_from_list(*list);
}

struct node* delete_item_from_list(struct node* head)
{
    struct node* iter = head;
    struct node* last = NULL;

    while (iter != NULL)
    {
        if (iter->x == x)
        {
            break;
        }

        last = iter;
        iter = iter->next;
    }

    if (iter == NULL)
    {
        printf("not found\n");
    }
    else if (last == NULL)
    {
        printf("found in first element: %i\n", x);
        head = iter->next;
    }
    else
    {
        printf("deleting element: %i\n", x);
        last->next = iter->next;
    }
   
    free(iter);
    return head;
}

Source Link

answered Feb 1, 2011 at 0:32

Loki Astari

97.7k
5
126
341

Loading

Stack Exchange Network

Return to Answer

General Comments:

Comments on Delete:

General Comments:

Comments on Delete: