CODEWITHSUNDEEP
c++algorithmlinked-list
Mahmoud Almansouri
Mahmoud Almansouri

Reputation: 13

insert node in a linked list runtime error

the code is supposed to add a node after the previous node argument, given the city name and the head pointer. I get a runtime error, however, when I run the code, why?

 city* addCity(city *head, city *previous, string cityName )
    {
        city* add = new city;
        add->name=cityName;
        add->next = NULL;
        city* tmp = new city;
        tmp = head;


        if(tmp==NULL){
            tmp = add;
        }

        while(tmp != NULL && tmp != previous){
            tmp = head;
            tmp = tmp->next;
        }


    if(tmp == previous){
        add->next = previous->next;
        tmp->next = add;
        head = tmp;
        return head;
        } 


        }

Upvotes: 1

Views: 194

Answers (4)

CiaPan
CiaPan

Reputation: 9570

Too much complication.

You have two general cases: either you're given the previous node in the list, so you don't have to seek it, or previous == NULL and you insert the new node at the front of the list (which also covers the case of an empty list).

city* addCity(city *head, city *previous, string cityName)
{
    city *newcity = new city;
    newcity->name = cityName;

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

    return head;
}

Note the return statement – your function has no access to the original head of the list, so the only way to inform the code outside that a new node appeared at the front of the list is returning a pointer to it. Of course, if a new node is inserted somewhere inside the list or appended at the end, the head doesn't change and the returned value equals the input head.

Upvotes: 0

Sushin Pv
Sushin Pv

Reputation: 1894

If you have the previous node that contain the next the node and the String you can directly add the new node to that.

Try this

city* addCity(city *head, city *previous, string cityName ){
    city* add = new city;
    add->name=cityName;
    if (head == NULL || previous == NULL){
        add->next = head ;
        return add;
    }
    add->next = previous->next;
    previous->next = add;
    return head;
}

This is enough too achieve the same ans.

For the first node to create check the following example

void Cites(){
    city *head = new city();
    head = addCity(head, head ,"First city name");
    //Do the rest of the codes
}

Edited: Now this code will work even to add node in the first. by passing previous as NULL

Upvotes: 0

Seeker
Seeker

Reputation: 126

Your code has a lot of issues.

Firstly,

city* tmp = new city;
tmp = head;

Your are creating a new city for tmp but immediately assigning it to head. Assuming you just need a *tmp, so you can change city *tmp = new city; to city *tmp = head;

Then,

if(tmp==NULL){
    tmp = add;
}

tmp == NULL means the list is empty. So you should add the new city to head and simply return from there. So, your code should be-

if(tmp==NULL){
    head = add;
    return head;
}

Continuing,

while(tmp != NULL && tmp != previous){
    tmp = head;
    tmp = tmp->next;
}

This is an infinite loop if tmp is not NULL. Because your are doing the same things in each iteration. It should be like-

tmp = head;

while(tmp != NULL || tmp != previous){
    tmp = tmp->next;
}

Forwarding,

if(tmp == previous){
    add->next = previous->next;
    tmp->next = add;
    head = tmp;
    return head;
}

I don't know why you are assigning tmp to head. Just removing head = tmp; should solve a lot of issues.

Upvotes: 0

Saurav Sahu
Saurav Sahu

Reputation: 13924

  while(tmp != NULL && tmp != previous){
        tmp = head;
        tmp = tmp->next;
  }

This will run infinite times as tmp is reset to head in each iteration. tmp is just toggling in a cyclic way between values head and head->next in this loop.

Upvotes: 1

Related Questions