Student Classroom class

Question

I'd like to know if the following class design is good or bad:

#include <iostream>
#include <vector>
#include <string>
#include <map>
using namespace std;
struct Student {
    string Firstname;
    string Surname;
    string IDNumber;
    int Age;
};

class Exam {
    string ExamNo;
    int Questions;
    int PassScore;
    string Subject;
    map <string,string> ExamTaken;
public:
    Exam(string exam,int Q , string s) : ExamNo(exam), Questions(Q),  Subject(s) {};
    void TakeExam(const string S) { ExamTaken[S] = ExamNo; };
};  

class Teacher {
    int studentscore;
    string ClassRoom;
public:
    bool StudentPassed ();
    void setScore (int studentsc) ;
};

class ClassRoom {
    vector<Student> students;
    string ClassName;
    double ClassAverage;
public:
    ClassRoom(string classname) : ClassName(classname) {};
    void AddStudent(Student &s) { students.push_back(s); };
    void ListStudents() const;
    int StudentCount() const { return students.size(); };
    void StudentLookup () const;
    string getClassName() const { return ClassName; };
};
// list all students in class
void ClassRoom::ListStudents() const {
/* old c++ style iterator
    for (vector<Student>::const_iterator it = students.begin();
    it != students.end();++it){
        cout << it->Firstname << " , ";
        cout << it->Surname << " , ";
        cout << it->Age;
        cout << endl;
    }
*/
// c++11
    for (auto &x : students){
        cout << x.Firstname << ",";
        cout << x.Surname << ",";
        cout << x.Age << endl;
    }
}

int main(){
    Student a,b;
    ClassRoom c("CS1002");
    Exam e("A002", 200 , "Biology");

    a.Firstname = "John";
    a.Surname = "Doe";
    a.IDNumber = "8123295073081";
    a.Age = 30;

    e.TakeExam(a.IDNumber);

    b.Firstname = "Jane";
    b.Surname = "Doe";
    b.IDNumber = "8123225073281";
    b.Age = 27;

    c.AddStudent(a);
    c.AddStudent(b);

    c.ListStudents();
    cout << "There are " 
         << c.StudentCount() 
         << " students in the classroom " 
         << c.getClassName() << endl;

}   

Maybe the Teacher class should be removed and replaced with a database class, that keeps a record of grades for each student.

I was thinking of changing the student class and adding object composition, for the exam class. I'm not sure how to do it properly yet, so I'm still experimenting with that.

Answer 1

I don't think that

 std::map <std::string,std::string> exam_taken;

and

void take_exam(std::string student_no) { exam_taken[student_no] = exam_number; };

belong in exam class.

An exam doesn't care if it was taken or not. Instead, a school does.

So, my suggestion is to add another class called school that will hold lists of students,teachers and exams. It'll also implement all the functions that interact with different objects. 'take_exam' is one of these functions. I think void teacher::add_course (std::string course) could also be moved into a school class

Remarks for a new code version:
What bothers me is the fact that student has no relation to a course at all. So you may ask the student to take an exam in a course he/she didn't study at all.
Below are my suggestions:
Course class holds list of registered users

1. Register(student) method
2. GiveExam(exam) method - calls each registered student TakeExam method

Teacher class should expose

1. Register(course) method,
2. SetExam(course, exam)

School class should orchestrate the rest of the classes and do things like initialization (like you already doing) and assignment of teachers to courses and exams to courses (though Teacher class)

Answer 2

First of all I'd like to say that you did not provide guidelines, so I'm not completely sure what is required and what not.

ClassRoom

I would rename this to Course, since it's not really related to the room where it takes place. A course probably has a instance variable that identifies the location of the course "Building A4.4" for instance. It indeed holds a vector of students, a name. But I wouldn't store a class average, instead provide a method for calculating that getCourseAverage() for example. Perhaps also store the Teacher object that gives this course.

Teachers

Your Teacher class seems not logically constructed, it is a person. If I were you I would create a Person class and inherit your Teacher and Student class from it. A Person has a firstname, lastname , age. Probably they both have an id value. You can even go a step further and make a Employee class, that has Person as base class and of which Teacher inherits.

Instead of string ClassRoom, I would use composition and have it contain a list of Course object (courses that are given by that teacher).

Constructors

Instead of using

 a.Firstname = "John";
 a.Surname = "Doe";
 a.IDNumber = "8123295073081";
 a.Age = 30;

create a constructor like: CreateStudent(name, surname, id, age); This is more concise and reads easier.

You might take a look into using an abstract factory pattern in order to create your student/teacher objects.

Style

Normally one does not start member variables with a upper case. perhaps take a look at this

apart from that you seem to do pretty well, style wise. But I'm not a C++ expert, I'm certain other can help you further with this.

Answer 3

I've refactored the code based on advice given. The code now makes use of object composition and inheritence.

Learned that objects should have class methods that are specific to the class. As an example a student takes an exam (composition). A school has teachers and students (composition).

Don't add members to a class that doesn't make sense. Example exam_taken for a class exam.

Herewith the code:

#include <iostream>
#include <vector>
#include <string>
#include <map>

class exam {
    int exam_questions;
    int exam_passscore;
    std::string exam_number;
    std::string subject;
public:
    exam() : exam_questions(0), exam_passscore(0), exam_number(""), subject("") {};
    exam(std::string number,int questions, std::string subject) : 
        exam_number(number), exam_questions(questions),  subject(subject) {};
};  


class person 
{
    std::string firstname;
    std::string surname;
    int idno;
    int age;
public:
    person(std::string firstname, std::string surname, int idno, int age): 
           firstname(firstname), surname(surname), idno(idno), age(age) {};
    std::string get_firstname () const { return firstname; }
};


class student : public person {
    std::string student_no;
    exam e;
public:
    student (std::string student_no, std::string firstname, std::string surname, int idno, int age) :
        student_no(student_no), person(firstname, surname, idno, age) { };
    std::string get_studentno () const { return student_no; };
    void take_exam(std::string examno, int questions, std::string subject);
};

void student::take_exam(std::string examno, int questions, std::string subject) 
{
    exam e(examno, questions, subject);
}


class teacher : public person {
public:
    teacher (std::string firstname, std::string surname, int idno, int age) :
        person(firstname, surname, idno, age) {};
};

class school {
    std::vector<student> students;
    std::vector<teacher> teachers;
    std::vector<std::string> courses;
    std::string school_name;
public:
    void add_course (std::string course); 
    school(std::string school_name) : school_name(school_name) {};
    void add_teacher (teacher &teach);
    void add_student (student &stud);   
};

void school::add_teacher (teacher &teach)
{
    teachers.push_back(teach);
}

void school::add_student (student &stud)
{
    students.push_back(stud);
}


int main(){
    student first("20012EE", "John", "Doe", 882123123,15);
    teacher t("Jane", "Doe", 9992123, 25);

    school s("The School");

    s.add_student(first);
    s.add_teacher(t);

    first.take_exam("T-2002", 200, "Biology");

    std::cout << first.get_firstname() << '\n';
}