Friend Classes / Operator Overloading

[cplushelp]

I have 2 classes, Student and Teacher. First we have to set grades for
the students (in the student class), then using friend, the Teacher class adds up the private grades.Then, using the student class again, we have to compare two students grades to see if they are the same. This is done using operator overloading with a == function.

So here is my problem, for some reason the final grade does not get passed back from the teacher class to the student class to compare. If I calculate the grade in the student class function, everything works fine. So I know my operator overloading function (bool Student::operator==) works. But because the project says we have to add the grades in the teacher function, it doesnt want to work...

If I need to clairify anything let me know. Thanks for any help!

#include<iostream> 
using namespace std;


// ---------Student Class-----------------

class Student 
{

private:

	int Id;			     // the id number of the student
	int FinalGrade;		 // the student's overall grade
	float Grade[4];		// four different test grades which range from 0 to 25


public:


	void setId(int x);          // sets the id number of the student

	void setGrade(float x[]);   // sets the four different test grades
	
	bool operator ==(const Student otherStudent);			//compares if the FinalGrade of the current 
	//student is equal to the FinalGrade of another student
	friend class Teacher;					
// allows the class Teacher to access private data members of student
	



};

// --------Member Functions for Student Class---------------



void Student::setId(int x) 
// set the private variable studentid to the passed integer
{
	Id=x;
}

void Student::setGrade(float x[]) 
// set the private array Grade to the passed array
{
	for(int i=0; i<4; i++)
	{
		Grade[i]=x[i];
	}
	
      //FinalGrade= Grade[0]+ Grade[1]+ Grade[2]+ Grade[3];    
       // if adding the grade is put here, everything works

}



bool Student::operator== (const Student otherStudent ) 
// Pre: self is initialized. otherStudent is initialized.
// Post: Returns true if self grade is identical to otherStudent
// numerically. Otherwise returns false.
{   

	// cout << otherStudent.Finalgrade;   // this is a garbage value, why??
        // cout << Finalgrade;               // this is a garbage value, why??

	if (FinalGrade== otherStudent.FinalGrade)
	return true;
	else
	return false;
}
// --------End Member Functions for Student Class---------------



// ---------Teacher Class-----------------

class Teacher 
{

private:

	int Id;			     // the id number of the student
	float salary;		// teacher's salary


public:


	void calculateGrade(Student x);          
// adds the four grades from class student to the final grade

};


void Teacher::calculateGrade(Student x)
{
	Student first;
      
   x.FinalGrade= x.Grade[0]+ x.Grade[1]+ x.Grade[2]+ x.Grade[3];   
// add up the grades

   //cout << x.FinalGrade;			
// this is the right grade, but it does not show up in the student class
}

// ---------End Teacher Class Functions -------------


int main( )
{

//These values are used for test case #1:
float g1[] = {10.0, 20.0, 25.0, 25.0};
float g2[] = {25.0, 20.0, 10.0, 25.0};

/*
These values are used for test case #2:
float g1[] = {20.0, 40.0, 05.0, 15.0}; 
float g2[] = {25.0, 20.0, 10.0, 25.0};
*/

Student x, y;
x.setGrade(g1);
y.setGrade(g2);

Teacher z;
z.calculateGrade(x);
z.calculateGrade(y);
if(x==y)
cout << " Students x and y have the same grade!" << endl;
else
cout << " Students x and y don't have the same grade!" << endl;


return 0;
}

[magicalblender]

When I ran your program, I got
Students x and y don't have the same grade!
... And I thought, "shouldn't calculategrade()'s argument be pass-by-reference (or whatever they call it when you use ampersands)?" Like so:

	void calculateGrade(Student &x);          
// adds the four grades from class student to the final grade

};


void Teacher::calculateGrade(Student &x)
{

That way, when you use calculategrade, the answer "sticks".
So I compiled again and then the output was
Students x and y have the same grade!
Oh good :)

Does that help at all?

[ropez]

There are several problems with the design:

1:
As the previous poster points out, calculateGrade should take a reference to a student. If not, it gets it's own local copy. Changing the grade has no effect outside the scope of the function. To prevent this kind of "invisible" bugs, you should consider dissallowing copy operations on the student class, by adding these lines:

private:
    Student(const Student&);
    Student& operator=(const Student&);

2:
The comparison operator should take a const reference parameter, like this: (otherwise it won't work after the previous change)

bool Student::operator== (const Student &otherStudent ) 

3:
Overloading the comparison operator in this case might actually be a bad design. It's not obvious that student1 == student2 will compare the grades. It looks like the students themselves are compared. A function named e.g. compareGrades might be better.

4:
Friend classes like this should be avoided. It would be better to add a setter for the FinalGrade, and call it from the Teacher class.

5:
The student class has a redundancy issue, because the value of FinalGrade is dependant on the other grade values. I would consider making the FinalGrade a read-only attribute calculated on-the-fly, something like this:

int Student::getFinalGrade() const {
    return static_cast<int>(Grade[0]+Grade[1]+Grade[2]+Grade[3]);
}

This would of course break the requirement that the teacher should calculate the final grade. So why not just move the function to the teacher class, and let it return the grade, like this:

int Teacher::calculateGrade(const Student &x) const {
    return static_cast<int>(x.getGrade(0)+x.getGrade(1)+x.getGrade(2)+x.getGrade(3));
}

Then change the main function like this:

    Student x, y;
    x.setGrade(g1);
    y.setGrade(g2);

    Teacher z;
    int fg1 = z.calculateGrade(x);
    int fg2 = z.calculateGrade(y);
    if(fg1==fg2)
        cout << " Students x and y have the same grade!" << endl;
    else
        cout << " Students x and y don't have the same grade!" << endl;

Keep it simple! This gets rid of the overloaded operator, the friend relationship, the FinalGrade field, and it removes confusion.

[ropez]

One more thing:

In the code you posted, the id field on both the students and teacher object is uninitialized. It will have some random value. This is best avoided by implementing a default constructor that sets the id to some default value, like 0 or -1. (Or maybe automatically assigns unique ids using a counter).

Alternatively, the constructor might take an id as a parameter, so that it can be provided by the user.

// 1
Student::Student() {
    id = 0;
}

// 2
Student::Student(int id_) {
    id = id_;
}

// 3
Student::Student() {
    static count = 0;
    id = count++;
}