0

I want to create a function that creates an array of structures. What I have done is the following:

struct Student {
    char studentNames[128]; 
    unsigned int FN; 
    short selectiveDisciplinesList[10]; 
    unsigned short countOfSelectiveDisciplines; 
    bool hasTakenExams;
};
    
void fillInStudentInfo(Student &student) {...}

Student createStudentsArray() {
    unsigned int countOfStudents;
    std::cout << "Enter the number of students You are going to write down: " << std::endl;
    std::cin >> countOfStudents;
    Student studentsArray[countOfStudents];

    for (int i = 0; i < countOfStudents; ++i) {
        fillInStudentInfo(studentsArray[i]);
    }

    return *studentsArray;
}

However, when in main(), I assign:

Student studentArray = createStudentsArray();

I can not access the members of each different struct (f.e. studentArray[i].something).

Where does the issue come from?

If I do it like this:

Student * createStudentsArray() {
    ...
    return studentsArray;
}

with

Student * studentArray = createStudentsArray();

My IDE warns me:

Address of stack memory associated with local variable returned

But I do seem to be able to access the members of each struct. Is that a bad thing?

trincot
  • 317,000
  • 35
  • 244
  • 286
Nik
  • 37
  • 6
  • 3
    please please use std::vector, this is what its for – pm100 Mar 01 '22 at 01:26
  • On how and why you can return `studentsArray` and observe it seeming to work: [Can a local variable's memory be accessed outside its scope?](https://stackoverflow.com/questions/6441218/can-a-local-variables-memory-be-accessed-outside-its-scope) – user4581301 Mar 01 '22 at 01:53

1 Answers1

2

You really cannot do what you are trying to do in that way:

Student createStudentsArray() {
    ...
    Student studentsArray[countOfStudents];
    ...
    return *studentsArray;
}

Firstly, this is using a variable length array, which strictly speaking is invalid C++. My compiler, for example, doesn't allow it.

Second, it will only return one Student. Even if it returned them all, its very inefficient because it would have to copy them all, but it doesn't.

You tried this:

Student *createStudentsArray() {
    ...
    Student studentsArray[countOfStudents];
    ...
    return studentsArray;
}

This is even worse. The memory allocated to that array is released once the function ends. This is what the compiler warned you about. Things seemed to work afterwards, but Very Bad Things(tm) will happen very soon afterwards.

Most experienced C++ devs will do this:

std::vector<Student> createStudentsArray() {
    ...
    std::vector<Student> students(countOfStudents);
    ...
    return students;
}

If you "are not allowed to use vector" (StackOverflow sees that all the time), then do:

Student *createStudentsArray() {
    ...
    Student *studentsArray = new Student[countOfStudents];
    ...
    return studentsArray;
}

But you must then delete[] that array once you are finished using it.

Footnote - for extra info, if you are really interested.

In fact, the experienced devs would do this:

std::vector<std::shared_ptr<Student>> createStudentsArray() {
    ...
    std::vector<std::shared_ptr<Student> students(countOfStudents);
    for (int......)
    {
        students.push_back(std::make_shared<Student>());
        ...
    }
    return students;
}

Now, the vector contains a bunch of smart pointers rather than the actual objects, that is the most efficient way to make arrays (or other containers) of largish objects. std::vector and std::shared_ptr will clean up all the memory for you automatically.

Remy Lebeau
  • 555,201
  • 31
  • 458
  • 770
pm100
  • 48,078
  • 23
  • 82
  • 145
  • 1
    I dunno about the `shared_ptr` riff. I've picked up a somewhat pathological hatred of them used for anything other than shared ownership. – user4581301 Mar 01 '22 at 01:51
  • 2
    I don't know anyone who would use `vector` over `vector` in this kind of situation, unless `Student` was a polymorphic base class. And even then, `std::unique_ptr` would almost always be preferred over `std::shared_ptr` – Remy Lebeau Mar 01 '22 at 02:23
  • You got the point about 'not being allowed to use vectors' correct haha. Still haven't got to use them. Using 'new' for dynamic array actually makes lots of sense - it's the same for any other int/char/etc array. Thank you for the explanation! – Nik Mar 01 '22 at 13:57
  • @RemyLebeau In this situation yes, since the size is known in advance and very little fiddling about is done. But once you start doing things that require moving things around or if the possibility exists of moving things around then this.. So this is my standard pattern for all large scale shipping c++ code. Maybe unique_ptr instead tho. – pm100 Mar 01 '22 at 17:45