C++ - Array of struct pointers

I have a small program in which I have two structs: Person - Consists of id, and basic methods Ppl - Consists of an array of people with some methods to operate on the array.

struct Person {
 const int id;
 Person();
 Person(int a);
};

Person::Person(int a) : id(a) {}

This is the Person struct with its methods.

const int MAX = 5;

Sets limit on array length

struct Ppl {
 static int current;   //Keeps track of current position in array
 Person *ppls[MAX];    //Creates an array of Person structure pointers
 void add(int a);      //Adds a person with id to the next available position
 //void remove(int b);
 int searchid(int c);  //Searches ppls for an id c.
 Ppl();                //Constructor
};

int Ppl::current = 0;  //Initializing static variable

void Ppl::add(int a) {
 int ret = searchid(a);          //Determine if id a already exists in ppls
 //cout << "Ret: " << ret << endl;
 if (ret > MAX) {                //If value returned is > MAX, value exists
  cout << "User " << a << " already exists" << endl;
 } else {                        //else, add it to the next available spot
  Person p(a);
  ppls[current] = &p;
  cout << "Added user: " << a << " at index: " << current << endl;
  current++;
 }
}

Ppl::Ppl() {
 current = 0;
 int i = 0;
 while (i < MAX) {    //Sets all Person pointers to NULL on initialization
  ppls[i] = NULL;
  cout << "ppls[" << i << "] is NULL" << endl;
  i++;
 }
}

int Ppl::searchid(int c) {
 int i = 0;
 bool found = false;
 while(i < MAX) {
  if (ppls[i] == NULL) {  //If NULL, then c wasn't found in array because
   break;                 //there is no NULL between available spots.
  } else {
    if (ppls[i]->id == c) {
     found = true;
   }
  }
  i++;
 }
 if (found == true) {
  return 10;     //A number higher than MAX
 } else {
  return 1;      //A number lower than MAX
 }
}

The main function is:

int main() {
 Ppl people;
 people.add(21);
 cout << people.ppls[0]->id << endl;
 people.add(7);
 cout << people.ppls[0]->id << " ";
 cout << people.ppls[1]->id << endl;
 people.add(42);
 cout << people.ppls[0]->id << " ";
 cout << people.ppls[1]->id << " ";
 cout << people.ppls[2]->id << endl;
 people.add(21);
 cout << people.ppls[0]->id << " ";
 cout << people.ppls[1]->id << " ";
 cout << people.ppls[2]->id << " ";
 cout << people.ppls[3]->id << endl;
}

The output that I get is:

ppls[0] is NULL
ppls[1] is NULL
ppls[2] is NULL
ppls[3] is NULL
ppls[4] is NULL
Added user: 21 at index: 0
21
Added user: 7 at index: 1
7 0
Added user: 42 at index: 2
42 0 0
Added user: 21 at index: 3
21 0 0 0

Why is it adding all new entries to the beginning of the array while keeping the rest NULL? Why isn't it detecting that 21 was already added.

I have been going crazy trying to figure this out. Any help would be much appreciated. Thanks a lot community.

EDIT I have fixed it so that it adds the elements to the array and recognizes when an id exists.

I made changes to the Ppl struct by adding a destructor:

Ppl::~Ppl() {
 int i = 0;
 while (i < MAX) {
  delete ppls[i];
  i++;
 }
}

and by changing the add method.

void Ppl::add(int a) {
 int ret = searchid(a);
 //cout << "Ret: " << ret << endl;
 if (ret > MAX) {
  cout << "User " << a << " already exists" << endl;
 } else {
  **Person *p = new Person(a);
  ppls[current] = p;**
  cout << "Added user: " << a << " at index: " << current << endl;
  current++;
 }
}

So the output now is

ppls[0] is NULL
ppls[1] is NULL
ppls[2] is NULL
ppls[3] is NULL
ppls[4] is NULL
Added user: 21 at index: 0
21
Added user: 7 at index: 1
21 7
Added user: 42 at index: 2
21 7 42
User 21 already exists
Segmentation fault (core dumped)

What is a segmentation fault and how can I fix it? Thanks again

Upvotes: 3

Views: 3768

Answers (1)

R Sahu
R Sahu

Reputation: 206737

Person p(a);
ppls[current] = &p;

is a problem. You are storing a pointer to a local variable. Your program is subject to undefined behavior.

Use

Person* p = new Person(a);
ppls[current] = p;

Make sure to delete the objects in the destructor of Ppl.

Suggestion for improvement

It's not clear what's your objective with this program but you can make your life a lot simpler by using

std::vector<Person> ppls;

instead of

Person *ppls[MAX];

Upvotes: 4

Related Questions