C++ Student Class

Question

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 <string>
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 <iostream>
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 <iostream>
#include "Student.h"
using namespace std;

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

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

Answer 1

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<<

Consider overloading operator<< for Student so that it'll be easier to output.

Declare it in the header:

friend std::ostream& operator<<(std::ostream& out, Student const& obj);

Define it in the implementation:

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;
}

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 << "Enter student information (ID, name, GPA, gender):";
std::cin >> 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.

Answer 2

It's a fine starting point.

Overall Design

But if I was to classify it at the moment it really a property bag (a set of properties with no associated actions (thus not a class)). A class is a set of properties that have meaning together. The state of an object of that class is manipulated by actions that are associated with the class.

Getter/Setters

This is why I also hate Getters/Setters.

   int getID();
    string getName();
    double getGPA();
    char getGender();

They provide no intrinsic value and expose implementation details about your class. Sure there are sometimes reasons to have them but usually it is better to provide appropriate methods that manipulate the state of the class rather the just giving access to the members.

Extended Setter

I see no reason for a set method:

void Student :: setStudent(int ID, string name, double GPA, char gender)

What is the use case?

Student   loki(5, "Loki", 4.8 /* So much better than you would expect*/, 'A');


// Change the value of the variable?
loki.setStudent(8, "Deepak", 1.0, 'M');

Sounds like a resonable use case. But it has so many detractors (You have to manually set all the values). Why not just set if from another student object.

Student   loki(5, "Loki", 4.8 /* So much better than you would expect*/, 'A');

loki = Student(8, "Deepak", 1.0, 'M');  // Reset loki to a different student.

Default construction: Hmmmm

Student :: Student()
{
    ID = 0;
    name = "";
    GPA = 0;
    gender = ' ';
}

After construction an object is supposed to be in a valid state. But this does not look very valid to me. When you write other code you make assumptions about the objects you use. One of these assumptions is that the objects are in a valid state so you can just use them (you don't need to validate them because the object validated itself on construction and any members validate that mutations are correct).

What if I write a print function:

 void printGenScore(Student const& s1)
 {
      std::cout << "I am " << s1.getGender() << " - " << s1.getGPA() << "\n";
 }

If I use this with the default student:

 Student s1;
 printGenScoew(s1);

 // I get the output
 // I am  - 
 //

Does not look like a big problem. As a human I can read that as a blank user. But now you are assuming the text will be read by a human and not some other piece of code that expects the input to be in a very precise format as another group has written the code:

 void scoreByGenederBigDataParser(std::istream& f)
 {
     std::string  res;
     std::char    gender;
     float        score;

     f >> res >> res >> gender >> res >> score;
     data[gender] += score;
 }

Now all hell breaks loose as their assumptions have been broken.

Review of C++

Lets stop doing this.

using namespace std;

Its lazy and leads to bad habbits. There issues associated with bringing stuff into the current namespace. So don't do it. Look up why.

std::cout << "Hi\n"; // Its not that hard to add `std::` to stuff.

Prefer Initializer lists

Use the initializer list from the constructor. And don't bother with this-> its not very common in C++ circles.

Some people say if you use this-> it prevents errors. True; but then you are only relying on your eyes rather than the compiler to find errors.

If you turn the warning level of your compiler up and treat warnings as errors. Then you get even better error protection and you don't need the ugly this-> as it does not help you find errors anymore (as the compiler is doing it for you).

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

Note Getters don't change the state. So make them const.

Student::print

Don't limit your printing to std::cout. Pass a stream that defaults to std::cout but allows the user to pass the stream type they want.

Don't use std::endl (prefer "\n") unless you want to flush the buffer. You usually don't want to flush the buffer. Flushing is an efficiency thing and the standard flush cycle is designed to make the stream very efficient. If you force a flush you will probably make the code slower (unless you know what you are doing). For I/O operations with the user std::cin and std::cout are synced. So if you ask a question the output is automatically flushed so they can read it before providing an answer.

print does not alter the state of the object. So it should be marked as const

class Student
{
    void print(std::ostream& str = std::cout) const
    {
        str << "ID : " << ID << "\n"
            << "Name : " << name << "\n"
            << "GPA : " << GPA << "\n"
            << "Gender : " << gender << "\n"
    }
};

Now you can also add the standard stream operator that most people in C++ use.

std::osteam& operator<<(std::ostream& str, Student const& data)
{
     data.print(str);
     return str;
}

Student   loki(5, "Loki", 4.8, 'A');
std::cout << loki;

Answer 3

Student.h specification file

Did you mean header?

• You should not use using namespace std in header files: use std:: instead.
• You don't need two constructors: you can use default parameters.
• In function prototypes you could omit the names, because the compiler will ignore them.

Other things I'd change, IMHO:

• Take advantage of class default access specification: remove the private specifier since they're already private.
• If you allow the user to set data through setters but you don't check what he's actually putting, there's no difference if they were directly public.

Student.cpp

• In print, you'd better to flush the buffer only once
• Pass string as const &, if you aren't going to modify it.
• Consider using the operator<< in order to make the output process sweeter. (you could just call print inside)

Blockquote StudentDemo.cpp

• Where main resides, the file should be named main.cpp

If you follow all my advices, you'll get:

Student.h

#ifndef STUDENT_H
#define STUDENT_H

#include <string>
using std::string;

struct Student
{
        Student(int ID = 0, const string &name = " ", double GPA = 0.0f, char gender = ' ');
        void setStudent(int ID, string name, double GPA, char gender);
        void print();

    int ID;
    string name;
    double GPA; 
    char gender;

};
#endif

Student.cpp

#include "Student.h"
#include <iostream>
using namespace std;

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

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

main.cpp

#include <iostream>
#include "Student.h"
using namespace std;

int main()
{
    Student s;

    cout << "Enter ID ";
    cin >> s.ID;

    cout << "Enter name ";
    cin >> s.name;

    cout << "Enter GPA ";
    cin >> s.GPA;

    cout << "Enter gender ";
    cin >> s.gender;

    s.print();
    return 0;
}  

Answer 4

At least you should have such member function declarations.

int getID() const;
string getName() const;
double getGPA() const;
char getGender() const;
void print() const;

By adding const specifers can make const objects access the member function.

In addition, if you focus on exception safety. You should add noexcept specifer on depends which indicates the function won't throw any exception.