HiveBrain v1.2.0
Get Started
← Back to all entries
patterncppModerate

C++ Student Class

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
studentclassstackoverflow

Problem

Is this a good approach of designing a class or is there some other way that I am not aware of?

Student.h
specification file

#ifndef STUDENT_H
#define STUDENT_H

#include 
using namespace std;

class Student
{
    private:
        int ID;
        string name;
        double GPA; 
        char gender;
    public:
        Student();
        Student(int ID, string name, double GPA, char gender);
        void setStudent(int ID, string name, double GPA, char gender);
        int getID();
        string getName();
        double getGPA();
        char getGender();
        void print();
};
#endif


Student.cpp
implementation file

#include "Student.h"
#include 
using namespace std;

Student :: Student()
{
    ID = 0;
    name = "";
    GPA = 0;
    gender = ' ';
}
Student :: Student(int ID, string name, double GPA, char gender)
{
    this -> ID = ID;
    this -> name = name;
    this -> GPA = GPA;
    this -> gender = gender;
}

void Student :: setStudent(int ID, string name, double GPA, char gender)
{
    this -> ID = ID;
    this -> name = name;
    this -> GPA = GPA;
    this -> gender = gender;
}

int Student ::  getID()
{
    return ID;
}

string Student :: getName()
{
    return name;
}

double Student :: getGPA()
{
    return GPA;
}

char Student ::  getGender()
{
    return gender;
}

void Student :: print()
{
    cout << "ID : " << ID << endl;
    cout << "Name : " << name << endl;
    cout << "GPA : " << GPA << endl;
    cout << "Gender : " << gender << endl;
}


StudentDemo.cpp

#include 
#include "Student.h"
using namespace std;

int main()
{
    Student s;
    int ID;
    string name;
    double GPA; 
    char gender;

    cout > ID;
    cout > name;
    cout > GPA;
    cout > gender;
    s.setStudent(ID, name, GPA, gender);
    s.print();
    return 0;
}

Solution

Student.cpp

Initializer lists

You should use initializer lists in place of these constructors. They offer some advantages, such as being able to initialize const members. Normal constructors do not allow that.

The initializer list is in this form:

Class::Class() : member(value) /* additional members separated by commas */ {}


Here's what it'll look like with the overloaded constructor:

Student::Student(int ID, std::string const& name, double GPA, char gender)
    : ID(ID)
    , name(name)
    , GPA(GPA)
    , gender(gender)
{}


You should then apply the same idea to the other constructor.

Overloading operator>

You can also do the same with operator>> to make inputting cleaner.

Declare it in the header:

friend std::istream& operator>>(std::istream& in, Student& obj);


Define it in the implementation:

std::istream& operator>>(std::istream& in, Student& obj)
{
    return in >> obj.ID >> obj.name >> obj.GPA >> obj.gender;
}


Regarding name: you should use std::getline() instead of std::cin >> so that spaces can be properly handled (it's also preferred in general for inputting into an std::string). However, it'll require a call to std::ignore() as you cannot just mix both forms of input.
StudentDemp.cpp

You should construct the Student object instead of using a setter.

If you keep your user inputs as-is, this is how the construction should look:

Student s(ID, name, GPA, gender);


But if you decide to use operator>>, you just need the default Student:

Student s;


and your input will now look like this:

std::cout > s;


With the latter, you should then remove the other variables from the top of the function.
Final notes:

-
Please do not use using namespace std in so many places. It can cause name-clashing issues, especially in larger programs. While it is okay to use it in a lower scope (such as a function), it's especially worse to use it in a header file. Any file including the header will be forced to use it, which could cause bugs. Read this for more information.

-
Declare variables as close in scope as possible to help ease maintainability. This is apparent with your variables declared at the top of main().

-
Avoid setters and getters as much as possible. They can break encapsulation as they expose the internals of the class. In this particular program, you won't even need them if you're just displaying values. Remove them all and it should make the code cleaner.

Code Snippets

Class::Class() : member(value) /* additional members separated by commas */ {}
Student::Student(int ID, std::string const& name, double GPA, char gender)
    : ID(ID)
    , name(name)
    , GPA(GPA)
    , gender(gender)
{}
friend std::ostream& operator<<(std::ostream& out, Student const& obj);
std::ostream& operator<<(std::ostream& out, Student const& obj)
{
    out << "ID: " << obj.ID << "\n";
    out << "Name: " << obj.name << "\n";
    out << "GPA: " << obj.GPA << "\n";
    out << "Gender: " << obj.gender << "\n";

    return out;
}
friend std::istream& operator>>(std::istream& in, Student& obj);

Context

StackExchange Code Review Q#49416, answer score: 12

Revisions (0)

No revisions yet.