Thread: Deleting elements from a linked list

Hi guys I m practicing with this problem :
"Write a program to remove an element from a linked list; the remove function should take just the element to be removed."
I m trying to do it personal. Before to get to that function (that I ve no idea of how to write it to be honest ) i' d like to create a program that let the user insert name and age (choosing it only between 20, 30, or 40 yo) of a person and store it in a structure. Afterwards, it should print all the names of these people and the user can choose which one to delete. End eventually reprint all their names (except the one deleted obviously).

Code:

#include <iostream>
#include <string>
using namespace std;
struct Group
{
    string name;
    int age;
    Group* p_next_element;
};
Group* head = NULL; // It will be always updated as the front of the list
Group* p_current = head; // I' ll need this pointer to the head of the list
                        // later on the main to print all the names
Group* Get20Years ()
{
    Group* person = new Group;
    person->age = 20;
    person->p_next_element = head;
    head = person;
    return person;
}
Group* Get30Years ()
{
    Group* person = new Group;
    person->age = 30;
    person->p_next_element = head;
    head = person;
    return person;
}
Group* Get40Years ()
{
    Group* person = new Group;
    person->age = 40;
    person->p_next_element = head;
    head = person;
    return person;
}
int main()
{
    int input;
    int age;
    Group* person;
    do{
        cout<<"How old is he/she? 20, 30, or 40?"<<endl;
        cin>> age;
        switch (age)
        {
            case 20: Group* Get20Years ();break;
            case 30: Group* Get30Years ();break;
            case 40: Group* Get40Years ();break;
        }
        cout << "What s his/her name?"<<endl;
        cin >> person->name; // it seems to be the problem....
        cout <<" Press 1 to see all the people set so far or any other key to keep adding"<<endl;
        cin >>input;
    }while ( input != 1);
    while (p_current != NULL)
    {
        cout<<p_current->name<< " " <<p_current->age<< " years old";
        p_current = p_current->p_next_element;
    }
}

This is what I got so far, but when I run the program and it comes to inset people's name the program quits itself.
Any help?
p.s. I m a very beginner and this is the knowledge I ve got so far... I know that probably many of you would have done it differently.
p.s.s. sorry if I said anything wrong.... my English may not be perfect as well

Originally Posted by Guest

Welcome to the forum! You're correct in where you assume the issue to be. person does not contain the data you expect it to.

Code:

Group* person;

// ...

switch (age)
{
    case 20: Group* Get20Years ();break;
    case 30: Group* Get30Years ();break;
    case 40: Group* Get40Years ();break;
}

you are damn right … that was really silly . Thanks a lot, but now I ve probably an even sillier problem but I m really stuck there... once I press 1 the program instead of printing out all the data it quits.... I ve tried to change something by adding a new function but nothing...

Code:

Group* DisplayData (Group* person)
{
    cout<<person->name<< " ";
    cout<<person->age<< " years old"<<endl;
}

int main()
{
    int input;
    int age;
    Group* person;
    do{
        cout<<"How old is he/she? 20, 30, or 40?"<<endl;
        cin>> age;
        switch (age)
        {
            case 20: person = Get20Years ();break;
            case 30: person = Get30Years ();break;
            case 40: person = Get40Years ();break;
        }
        cout << "What s his/her name?"<<endl;
        cin >> person->name; 
        cout <<" Press 1 to see all the people set so far or any other number to keep adding"<<endl;
        cin >>input;
    }while ( input != 1);
    while (p_current != NULL)
    {
        DisplayData(p_current);// well... now i ve a problem here...
        p_current = p_current->p_next_element;
    }
}

If your code is mostly as in the first example, your print loop will immediately evaluate to false:

Code:

Group* head = NULL;
Group* p_current = head; // so p_current is NULL

while (p_current != NULL) // evaluates to false (don't enter loop body)
{
    DisplayData(p_current);
    p_current = p_current->p_next_element;
}

Btw, in C++ you should use the nullptr literal everywhere, in place of the old NULL macro, e.g.:

Code:

Group* head = nullptr;

while (p_current) // You can skip the comparison expression btw, as null pointers evaluate to false
{
    // ...
}

Out of curiosity: Why so complicated ?
The task could easily be done like this:

Code:

#include <iostream>
#include <string>
#include <list>

using namespace std;

struct Group
{
  string name;
  int age;
};

int main()
{
  int input;
  int age;
  Group person;
  list<Group> people;

  do
  {
    cout << "How old is he/she? 20, 30, or 40?" << endl;
    cin >> age;
    cin.ignore(255, '\n'); // remove trailing '\n'
    switch (age)
    {
    case 20:
      person.age = 20;
      break;
    case 30:
      person.age = 30;
      break;
    case 40:
      person.age = 40;
      break;
    }
    cout << "What s his/her name?" << endl;
    cin >> person.name;
    people.push_back(person);

    cout << " Press 1 to see all the people set so far or any other key to keep adding" << endl;
    cin >> input;
    cin.ignore(255, '\n'); // remove trailing '\n'
  } while (input != 1);

  for (const Group& g : people)
  {
    cout << g.name << " " << g.age << " years old\n";
  }

}

Originally Posted by OldGuy2

Out of curiosity: Why so complicated ?
The task could easily be done like this:

Code:

#include <list>

using namespace std;

struct Group
{
  string name;
  int age;
};

int main()
{
  int input;
  int age;
  Group person;
  list<Group> people;


}

'cause I still know nothing about "list"... my book hasn t covered that topic yet, so I m supposed to do this exercise with what I learnt so far. Furthermore this chapter was about linked lists using pointers in structures, so I think I should use that for this exercise.

The problem now is that you're passing the head pointer by value, so the pointer gets copied into the GetXXYears functions and that copy is being assigned to.

To reach the head in the parent scope, you'd need a pointer-to-pointer, e.g.:

Code:

Group* Get40Years (Group** head)
{
    // head is now being dereferenced
    person->p_next_element = *head;
    *head = person;
}

case 40: person = Get40Years(&head); break; // address of head pointer

Honestly though, with your current approach the errors will keep piling up, guaranteed! It's good to eventually learn about pointers and their convoluted ways, but I feel this is the wrong kind of program to do that with. Simpler would be better.

Honestly though, with your current approach the errors will keep piling up, guaranteed!

Ok yeah you are right... I ve tried to change approach as well as I reviewed a few things about pointers which I didn t get very well, now my code looks like this:

Code:

#include <iostream>
#include <string>
using namespace std;
struct Group
{
   int age;
   string name;
   Group* next_element;
};
Group* GetNewElement (int years, string name, Group* head)
{
    Group* person = new Group;
    person->age = years;
    person->name = name;
    person ->next_element = head;
    person = head;
    return person;
}

int main ()
{
    int years;
    string name;
    int input;
    Group* person;
    Group** head = NULL;
    while (input != 1)
    {
       cout<< "Enter a person's name"<<endl;
       cin>>name;
       cout<< "Enter his/her age "<<endl;
       cin>>years;
       GetNewElement (years, name, *head); // this function seems the one which make my program stop working
       cout << " Press 1 if you want to show the list of the people set so far, otherwise press any other number to keep adding people"<<endl;
       cin>>input;
    }
    Group* p_current = *head;
    Group* p_prev = NULL;
    for (p_current = *head; p_current != NULL; p_current = p_current->next_element)
    {
        cout<<person->name<<" "<<person->age<<" years old"<<endl;
    }
}

Now when I run the program and I insert the name it quits immediately.... :/

As if I said nothing guys I eventually managed to do it and now it works as I expected... this is my code:

Code:

#include <iostream>
#include <string>
using namespace std;
struct Group
{
   int age;
   string name;
   Group* next_element;
};
Group* GetNewElement (int years, string name, Group* head)
{
    Group* person = new Group;
    person->age = years;
    person->name = name;
    person ->next_element = head;
    return person;
}
void List (Group* person)
{
    cout<<person->name<<" ";
    cout<<person->age<<" years old"<<endl;
}
void Deleting (string value, Group*  head)
{
   Group* p_current = NULL;
   Group* p_prev = NULL;
   for (p_current = head; p_current != NULL; p_prev = p_current, p_current = p_current->next_element)
    {
      if (p_current->name == value)
      {
          if (p_prev==NULL)
            head=p_current->next_element;
          else
            p_prev->next_element = p_current->next_element;
      }
    }
}

int main ()
{
    int years;
    string name;
    int input;
    Group* person;
    Group*  head = NULL;
    string Delete;
    while (input != 1)
    {
       cout<< "Enter a person's name"<<endl;
       cin>>name;
       cout<< "Enter his/her age "<<endl;
       cin>>years;
       person, head = GetNewElement (years, name, head);
       cout << " Press 1 if you want to show the list of the people set so far, otherwise press any other number to keep adding people"<<endl;
       cin>>input;
    }
    Group* p_current = head;
    Group* p_prev = NULL;
    while (p_current != NULL)
    {
        List(p_current);
        p_current=p_current->next_element;
    }
    cout<<"Select the one you want to delete from the list by his name"<<endl;
    cin >> Delete;
    Deleting (Delete, head);
    cout<<"This is your final list: "<<endl;
    p_current=head;
    while (p_current != NULL)
    {
        List(p_current);
        p_current=p_current->next_element;
    }
}

Could you only suggest me where I should deallocate all the memory I ve used?