c++ classes and garbage data

Question

For my last project this year I am trying to make a Gradebook program in C++. Using classes vs structs, new and delete instead of malloc and free, I am to recreate my previous homework written in C. In my last question someone told me to stop making my variables private as I have a pointer to that class from my main Class.

in gradebook.h

class Student {
 public: 
  string last;
  string first;
  int student_id;
  int count_student;
};

class Course {
 public:
   string name;
   int course_id;
   int count_course;

};

class Gradebook {
 public:
  Gradebook();
  void addCourse();
  void addStudent();
  void printCourse();
  void printStudent();

 private:
  Course *courses;
  Student *students;
 };

in gradebook.cpp

 Gradebook::Gradebook() {

  courses = new Course;
  courses->count_course=0;
  courses->course_id = 0;


  students = new Student;
  students->count_student=0;
  students->student_id=0;    

}

Gradebook::addCourse() {

 int i, loop=0;

 cout << "Enter number of Courses: ";
 cin >> loop;

 for(i=0; i<loop; i++) {

  cout << "Enter Course ID: ";
  cin >> courses[courses->count_courses].course_id;

  cout << "Enter Course Name: ";
  cin >> courses[courses->count_courses].name;

  courses->count_course++;

 }

}

 Gradebook::addStudent() {

   //same information from addCourse but goes to students variables

 }

Gradebook::printCourse() {

 int i;

 for(i=0; i<courses->count_course; i++) {

   cout << courses[i].course_id << "\t\t";
   cout << courses[i].name << endl;

 }

}

Gradebook::printStudent() {

 int i;

 for(i=0; i<students->count_student; i++) {

   cout << students[i].student_id << "\t\t";
   cout << students[i].last << "\t\t";
   cout << students[i].first << endl;

 }

}

When I run addCourse function then run printCourse it works.

Then I run addStudent function then run printStudent it works.

Problem:

After I add students and rerun printCourse I get garbage data when courses[i].course_id gets printed. But only when i=2. courses[i=2].name still prints with the correct data. I can add more courses and add more students and they print out just fine, again only when i=2 does course_id get garbage data. I've been stuck for a few days and I tried to look at it a different way until @wheaties mentioned that what I was doing previously was correct and it should be public. So can one of you guys help me out?

Answer 1

Your problem:

The following:

courses[courses->count_courses].course_id;

Does not do what you'd want it to do. It treats courses like an array of Course objects, when in reality it is just a single (heap-allocated) object. So as soon as count_courses is increased, you access memory beyond the single existing Course object, which, as you already observed, yields only garbage.

How to fix it?

The correct "C++" way would be using std::vectors instead of pointers for the Course and Student lists. But if you really have to go with manual memory allocation, you have to:

1. Move count_courses and count_students out of the corresponding classes and into CourseBook. Each student or course object represents a single course/student, so it doesn't make sense to store a count in it.
2. Allocate the memory using "array new": courses = new Course[count];. Of course, you have to move the allocation out of the constructor then, and do it in addCourse instead, where you know how many items will be there
3. Don't forget to release the allocated memory..

Answer 2

Problem:

in Gradebook::Gradebook(), you construct a single Course:

courses = new Course;
courses->count_course=0;
courses->course_id = 0;

and the same goes for Student. You need to construct either an array or use one of the STL's containers. For this case, I'd recommend either std::list or std::deque. A simple usage would be:

class Gradebook {
  std::list<Course> courses;
}

void Gradebook::addCouse() {
  int count = 0;
  cout << "Courses count: ";
  cin >> count;

  while (count-- > 0) {
    Course course;
    cout << "Course ID: ";
    cin >> course.id;

    cout << "Course name: ";
    cin >> course.name;

    courses.push_back(course);
  }
}

void Gradebook::printCourse() {
  cout << "There are " << courses.size() << " courses" << endl;
  for (std::list<Course>::iterator i = courses.begin(); i != courses.end(); ++i) {
    cout << "    ID: " << i->id << " name: " << i->name << endl;
  }
}

If you wish to use std::vector, only the addCourse would change:

void Gradebook::addCouse() {
  int count = 0;
  cout << "Courses count: ";
  cin >> count;
  courses.reserve(count);

  while (count-- > 0) {
    Course course;
    cout << "Course ID: ";
    cin >> course.id;

    cout << "Course name: ";
    cin >> course.name;

    courses.push_back(course);
  }
}

Answer 3

If there is a

Course * courses;

it means that you have a variable containing an address to a memory area containing a Course. You can access the memory area (after having established it using new or malloc) by writing

*courses = ...;

or

courses->name.

However, C (and C++) lets you apply indexing to this pointer:

courses[i]

which is nothing but

*(courses + i)

i.e., following the pointer you obtain by incrementing courses with i times the size of Course.

Consider that new Course provides memory for a single Course and then look at what your loop in addCourses does...

There's no need for dynamic memory allocation if you use std::vector.

Answer 4

There is only one object of courses class, which you are creating in the constructor of Gradebook::Gradebook()

You should try to create as many objects of courses as you want to insert. You could do that by

  Gradebook::addCourse() {

 int i, loop=0;

 cout << "Enter number of Courses: ";
 cin >> loop;
 //add these lines here and remove them form Gradebook constructor
 courses = new Course[loop];
 courses->count_course=0;
 courses->course_id = 0;
 //^^^^^

 for(i=0; i<loop; i++) {

 cout << "Enter Course ID: ";
 cin >> courses[courses->count_courses].course_id;

 cout << "Enter Course Name: ";
 cin >> courses[courses->count_courses].name;

 courses->count_course++;

 } 

}