Why do I have a run-time Error inserting values into classes in C++

Question

I would like some help in getting this sorted out. I have a class named Grades that contains a pointer to class Student.

class Grades {
    int numofstuds;
    Student** studs;
public:
    void insertgrades();
    void printgrades();
};

class Student {
    int id;
    int grade;
public:
    void getId();
    void getGrade();
    void printgrade();
};

I am trying to insert students into the array studs:

void Grades::insertgrades() {
    int i;
    cout << "How many students ?" << endl;
    cin >> numofstuds;
    studs = new Student*[numofstuds];
    for (i = 0; i < numofstuds; i++) {
        studs[i]->getId();
        studs[i]->getGrade();
    }
}

'insertgrades' is an inner function inside the class Grades. getId and getGrade are inner functions inside the class 'Student', they work well I have checked them:

void Student::getId() {
    do {
        cout << "Enter id" << endl;
        cin >> id;
    } while (id<0);
}
void Student::getGrade() {
    cout << "Enter grade:" << endl;
    cin >> grade;
}

this is the main:

int main() {
    Grades a;
    a.insertgrades();
    return 0;
}

As soon as I am trying to insert values into Id or grades I immediately have a run-time error. Please help! Isan.

Is this schoolwork? Because you really better use standard containers instead of raw pointer arrays. — StoryTeller - Unslander Monica, Feb 24 '16 at 15:56
Why are you using a double pointer? From the way you are using it you should just have `Student* studs;` — NathanOliver, Feb 24 '16 at 15:56
It is a school work actually. I have decided to use studs** because later on deleting/adding new students will be easier :) — Isan Rivkin, Feb 24 '16 at 15:57
You allocated an array of `Student*` with `new Student*[numofstuds]`, but those pointers are not valid. — crashmstr, Feb 24 '16 at 15:57
@IsanRivkin `because later on..` Later on, you should get the chance to learn how to use containers and not double stars. — PaulMcKenzie, Feb 24 '16 at 16:05
@ThomasMatthews can you give me more info so I can research that ? — Isan Rivkin, Feb 24 '16 at 16:08
Your IDE may have a debugger for C++, so you should search the documentation of your IDE. Otherwise search the internet for "C++ debugger" and also insert your platform. — Thomas Matthews, Feb 24 '16 at 16:42

score 2 · Answer 1 · answered Feb 24 '16 at 15:57

2

You created only "boxes" to store pointers to Student, but you didn't store valid pointers.

You have to create Students before using.

void Grades::insertgrades() {
    int i;
    cout << "How many students ?" << endl;
    cin >> numofstuds;
    studs = new Student*[numofstuds];
    for (i = 0; i < numofstuds; i++) {
        studs[i] = new Student(); // add this line
        studs[i]->getId();
        studs[i]->getGrade();
    }
}

answered Feb 24 '16 at 15:57

MikeCAT

73,922
11
45
70

Why even do that? We can just change it to `Student* studs;` in the class and now we do not have to levels of pointers and two level of deletions to have to maintain. – NathanOliver Feb 24 '16 at 16:01
@NathanOliver, well it's schoolwork, and the professor already decided that torturous use of raw `new` and `delete` is better than using standard containers. So may as well suffer on two levels. – StoryTeller - Unslander Monica Feb 24 '16 at 16:02
But you're using `new` incorrectly in this case. It's one thing to teach `new`, its another when you're told to use it like this. A *much* more better use of `new` would have you build a *real* dynamic array class. You learn `new` / `delete`, copy construction, assignment, etc. Much more educative than throwing `new` anywhere in code and hoping it works. – PaulMcKenzie Feb 24 '16 at 16:13
Your professor needs education. If he wants you to learn Pointers however - you may want to brush up on the fundamentals. I'm about to post an answer myself that may help – John Bargman Feb 24 '16 at 16:14

John Bargman · Accepted Answer · 2016-02-24T16:37:41.440

MikeCAT's answer is correct, however please take note that what you are doing is VERY likely to cause memory leaks. Obviously I haven't seen your class destructor - However unless you are carefully iterating over every pointer in Studs, calling delete [], then calling delete [] on Studs - You will end up in a big pile of leaked memory.

I'm going to assume from your use of the language you are more familair with reference counted languages like VisualBasic - in C++ allocating Memory with New provides no assurance of garbage collection. As your not storing numberofstuds you will never be able to free the memory allocated in MikeCAT's answer.

If you can make use of C++11, consider the following code snippet for a memory-safe way to handle this problem.

#include <memory>
#include <vector>
class Student;
class Grades {
    int numofstuds;
    std::vector<std::shared_ptr<Student> > studs;
public:
    void insertgrades();
    void printgrades();
};

and the insert grades routine:

void Grades::insertgrades() {
    int i;
    std::cout << "How many students ?" << std::endl;
    std::cin >> numofstuds;
    studs.resize(numofstuds);
    for (auto Each_Student : studs) 
    {
        Each_Student.reset(new student())
        Each_Student->getId();
        Each_Student->getGrade();
    }
}

If however you must use raw pointers (older compiler, or poor grading body), please consider the following destructor in addition to MadCat's answer

~Grades()
{
    for (i = 0; i < numofstuds; i++) 
    {
        delete studs[i];
    }
delete [] studs;
}

[/edit]

I'd like to add that your approach to storing the data isn't wrong however you can do it much more simply. Rather than creating an array of pointers to store the data - you can allocate an array of the objects instead.

class Grades 
{
    //snipped out everything else
    Student* studs;
}
void Grades::insertgrades() 
{
    int i;
    cout << "How many students ?" << endl;
    cin >> numofstuds;
    studs = new Student[numofstuds];
    for (i = 0; i < numofstuds; i++) {
        studs[i].getId();
        studs[i].getGrade();
    }
}

In C++ a pointer is "The address in memory of data", an array is "the address in memory of a set of data", as such the following code should help you comprehend why the above works.

char A;
//the following pointer will *point to the location of A*
char * A_p = & A;
char Array[100];
//this pointer will *point to the data set Array*
char * Array_p = Array;
//this pointer *points to a newly allocated set of data*
char * B_p = new char [100];
//indeed, we can also do this 

//Set A to first character of array
A = *Array_p
A = Array_P[0]
A = *Array
A = Array[0]
//all of these do the same thing, as *when using an array, you're really using a pointer

delete [] B_P;

I'd postulate the `std::unique_ptr` is a better fit here. There is no sharing of ownership. — StoryTeller - Unslander Monica, Feb 24 '16 at 16:19
I still don't know what is or how to use that library, I did forget to delete the pointers thanks for reminding me that, also thanks for the code snippets I am going to study right now that code. — Isan Rivkin, Feb 24 '16 at 16:19
@StoryTeller using a unique_ptr in this situation isn't easily done - Remember that unique_ptr isn't copyable. As such it cannot be stored in a stl container! — John Bargman, Feb 24 '16 at 16:28
@IsanRivkin The vector is basically an array that can be resized! Something c++ without the STL doesn't have. What your tutor is trying to do is teach you how to dynamically allocate memory (I'm actually going to amend my answer about that in a moment). Allocating memory with raw pointers is how you dynamically-create an array without the use of std::vector. — John Bargman, Feb 24 '16 at 16:30
@JohnBargman, actually it can. You just have to remember to move it when inserting. STL containers can handle `std::unique_ptr` quite well nowadays. It's not c++03 with `boost::unique_ptr` anymore :) — StoryTeller - Unslander Monica, Feb 24 '16 at 16:30
@StoryTeller Surely it doesn't matter if I std::move it into the container. As if the container resizes, or moves it will use the copy-constructor? — John Bargman, Feb 24 '16 at 16:38
@StoryTeller I stand corrected! I hadn't taken note that STL containers now handle movable objects. Thank you for the update. — John Bargman, Feb 24 '16 at 16:40

score 0 · Answer 3 · answered Feb 24 '16 at 16:06

Thanks for the immediate response! I have solved this thanks to @MikeCAT. I added another func to actually create a student:

Student* createstud() {
    Student* News = new Student;
    return News;
}

And then I change my func like that:

void Grades::insertgrades() {
    int i;
    cout << "How many students ?" << endl;
    cin >> numofstuds;
    studs = new Student*[numofstuds];
    for (i = 0; i < numofstuds; i++) {
        **studs[i] = createstud();**
        studs[i]->getId();
        studs[i]->getGrade();
    }
}

Not sure why is it bad idea to create Students** array, since I am planning to delete students in the future so it is easier to change pointers inside the array, no ?

Read up http://stackoverflow.com/questions/6462985/c-stl-vector-vs-array-in-the-real-world — StoryTeller - Unslander Monica, Feb 24 '16 at 16:08
Please do also take care to delete ALL allocated memory (see my answer). And take a look into std::shared_ptr, and the other new managed pointer-types. — John Bargman, Feb 24 '16 at 16:18

Why do I have a run-time Error inserting values into classes in C++

3 Answers3

[/edit]