CODEWITHSUNDEEP
clistlinked-list
buzzcarla
buzzcarla

Reputation: 303

LinkedList Delete End in C

I am trying to program a simple text editor in C and I am using LinkedList. I have problems with the deleteEnd function. Where did I go wrong?

#include <stdio.h>
#include <stdlib.h>
#include<conio.h>

i store the character, and the coordinates of the character in a structure like this:

struct node {
    struct node *previous;
    char c;
    int x;
    int y;
    struct node *next;
}*head;

This is called whenever a letter is typed in.

void characters(char typed, int xpos, int ypos)     //assign values of a node
{
    struct node *temp,*var,*temp2;
    temp=(struct node *)malloc(sizeof(struct node));
    temp->c=typed;
    temp->x=xpos;
    temp->y=ypos;

    if(head==NULL)
    {
        head=temp;
        head->next=NULL;
    }

    else
    {
        temp2=head;
        while(temp2!=NULL)
        {
            var=temp2;
            temp2=temp2->next;
        }
        temp2=temp;
        var->next=temp2;
        temp2->next=NULL;
    }
}

Print the new node if there are changes.

void printer()          //to print everything
{
    struct node *temp;
    temp=head;
    while(temp!=NULL)
    {
        gotoxy(temp->x,temp->y);
        printf("%c",temp->c);
        temp=temp->next;
    }

}

Now here, I do not know why the last element of the head won't be deleted.

void deletesEnd()
{
    struct node *temp,*last;
    temp=head;
    while(temp!=NULL)
    {
        last=temp;
        temp=temp->next;
    }
    if(last->previous== NULL)
    {
        free(temp);
        head=NULL;
    }
    last=NULL;
    temp->previous=last;
    free(temp);
}

This is where it all begins.

main()
{
    char c;         //for storing the character
    int x,y;        //for the position of the character
    clrscr();
    for(;;)
    {
        c=getch();
        x=wherex();
        y=wherey();
        if(c==0x1b)     //escape for exit
        {
            exit(0);
        }

        else if (c==8)  //for backspace
        {
            deletesEnd();
            // clrscr();
            printer();
        }

        else            //normal characters
        {
            characters(c,x,y);
            printer();
        } 

    }
}

Upvotes: 1

Views: 5243

Answers (5)

Some programmer dude
Some programmer dude

Reputation: 409166

For finding the last node in the list, the simplest is to loop until the next pointer is NULL.

For deletion at the end you also need to keep track of the next-to-last node, and it could look something like this:

struct node *prev, *curr;

for (prev = NULL, curr = head; curr->next != NULL; prev = curr, curr = curr->next)
    ;

/* `curr` is now the last node in the list, `prev` is the next-to-last node */
if (prev != NULL)
    prev->next = NULL;  /* Unlink the last node */
else
    head = NULL;  /* List was only one node long */

/* Free the memory for the last node */
free(curr);

If you properly kept the list double-linked the loop would be even easier, as you don't have to keep track of the next-to-last node anymore:

struct node *node;

for (node = head; node->next != NULL; node = node->next)
    ;

/* `node` is now the last node in the list */
if (node->previous != NULL)
    node->previous->next = NULL;  /* Unlink last node */
else
    head = NULL;  /* List was only one node long */

/* Free the memory for the last node */
free(curr);

And keeping track of the tail from the start will make this much simpler:

if (tail != NULL)
{
    struct node *node = tail;

    if (node->previous != NULL)
    {
        node->previous->next = NULL;  /* Unlink last node */
        tail = node->previous;
    }
    else
        head = tail = NULL;  /* Removing last node in list */

    free(node);
}

Upvotes: 1

giorashc
giorashc

Reputation: 13713

This line :

temp->previous=last;

will cause you problems since temp is null at that point already (since it was the termination condition in the while loop).

Change your method to :

void deletesEnd()
{
   struct node *temp,*last;
   temp=head;
   while(temp!=NULL)
   {
       last=temp;
       temp=temp->next;
   }
   if(last != NULL) {
       if (last->previous != null) { // Last element gonig to be deleted so the previous element if exists should point his next to null (end of list)       
           last->previous->next = null;
       }   
       free(last); // Free the last element in the list
   }
}

Upvotes: 0

someone
someone

Reputation: 1696

Linkedlist work with only two simple generic rules...

  1. First Make the link
  2. Second after make, Break the link.

Just check the condition...

  1. If you have empty list
  2. if list has only one node
  3. if it has more then one node

try these rules. I hope it will work for you.

Upvotes: 0

Dimitri Mockelyn
Dimitri Mockelyn

Reputation: 1545

Try this :

void deletesEnd()
{
struct node *temp,*last;
temp=head;
last = temp;
while(temp != NULL && temp->next!=NULL)
{
    last=temp;
    temp=temp->next;
}
if(last == temp)
{
    free(temp);
    head=NULL;
} else {
    free(last->next);
    last->next = NULL;
}

In your algorithm, you try to work with a NULL pointer, so you can't get to the previous node I think.

Upvotes: 1

No Idea For Name
No Idea For Name

Reputation: 11577

try this

void deletesEnd()
{
   struct node *temp,*last;
   temp=head;

   while(temp!=NULL)
   {
       last=temp;
       temp=temp->next;
   }

   if(last->previous== NULL)
   {
        free(temp);
        head=NULL;
   }

   last->previous->next = NULL;
   last=NULL;
   temp->previous=last;
   free(temp);
}

you see, you've put the last element in the list in last, and then did last=NULL; which just put null in last variable. temp is already NULL so temp->previous=last; has no meaning. you need that the last item's previous item will point to NULL as the line 

last->previous->next = NULL;

does

Upvotes: 0

Related Questions