Reputation: 173
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?
Upvotes: 0
Views: 1216
Reputation: 113
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++;
}
}
Upvotes: 0
Reputation: 1651
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);
}
}
Upvotes: 1
Reputation: 4519
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::vector
s instead of pointers for the Course
and Student
lists. But if you really have to go with manual memory allocation, you have to:
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.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 thereUpvotes: 1
Reputation: 31290
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
.
Upvotes: 0