# Grade Calculator (New OOP Version from one previous)

I'm trying to develop an OOP way of thinking. I was hoping someone would be kind enough to spare some of their valuable time to review my Grade Calculator OOP program. As always, I'd like to know what I've done well, what I should improve upon, and any suggestions on how I could perhaps improve what I have? By the way, I have a class called Class. I probably should prefix it with "cls" so as not to confuse. Treat this program as is supposed to be entered, I haven't error checked it. The point of this program is to develop in OOP.

   // Task 1.cpp : This file contains the 'main' function. Program execution begins and ends there.

#include <iostream>
#include <vector>
#include <algorithm>
#include <functional>
#include <numeric>
#include <string>
class TestPaper
{
public:
int m_scoreOutOf;
bool checkBoundary(int value, int boundary) {
if (value < 0 || value > boundary) {
std::cout << "Score must be between " << " 0 and " << boundary << ". Please try again.\n";
return false;
}
return true;
}

};
class Student {
private:
std::string m_name;
int m_scoreGot;
public:

Student(std::string name, int scoreGot)
:m_name(name), m_scoreGot(scoreGot){}

std::string getName() const { return m_name; }
int getScoreGot() const { return m_scoreGot; }
};

class Class {
private:
std::vector<Student>students;
public:
std::string name = "";
int scoreGot = 0;
std::cout << "Enter student name: ";
std::getline(std::cin >> std::ws, name);
do
{
std::cout << "\nWhat did " << name << " score?\nEnter a score between 0 and "
<< testPaper.m_scoreOutOf << ": ";
std::cin >> scoreGot;
} while (testPaper.checkBoundary(scoreGot, testPaper.m_scoreOutOf) == false);
students.push_back({ name, scoreGot });
}

std::vector<Student>& accessStudents() { return students; }
};

TestPaper m_testPaper;
Class m_ClassOfStudents;
public:
GradeCalculator(TestPaper testPaper, Class classOfStudents) :m_testPaper(testPaper), m_ClassOfStudents(classOfStudents) {}

std::cout << "2. Calculate class score\n";
std::cout << "3. Modify testpaper (haven't implemented this yet)\n";
}

auto sum = std::transform_reduce(m_ClassOfStudents.accessStudents().begin(), m_ClassOfStudents.accessStudents().end(), 0.0, std::plus<>(),
[&](auto& student) { return calculateGradePercentage(student); });
return sum / m_ClassOfStudents.accessStudents().size();
}
{
return static_cast<double>(student.getScoreGot()) / static_cast<double>(m_testPaper.m_scoreOutOf) * 100;
}
void DisplayResult() {
for (auto& student : m_ClassOfStudents.accessStudents()) {
std::cout << "Percentage scores are: \n";
std::cout << student.getName() << ": " << calculateGradePercentage(student) << "%\n";
}
}
void runProgram() {

while (true)
{
std::cout << "\nEnter a choice from the menu: ";
{
case 1:
break;
case 2:
DisplayResult();
break;
default:
std::cout << "Invalid choice!\n";
}
}
}
};

int main()
{
TestPaper testPaper({ 20 });
Class classOfStudents;
calculator.runProgram();

}


## Don't mix I/O with data operations

Generally speaking having a std::getline and std::cout within a data class such as Class is not a good idea. It makes it harder to reuse the class. Better practice is to keep the data (Student, Class, etc.) separate from getting input from users. The Model-View-Controller design pattern is often useful for programs like this. Consider using something like a ConsoleMenu class as in this answer.

## Use move semantics where practical

In the AddStudent function, we have this line:

students.push_back({ name, scoreGot });


Better would be to use emplace_back which tells the compiler that it doesn't need to construct and copy, but that it's safe to construct the object in place.

## Rethink the class interface

The Class class has this member function:

std::vector<Student>& accessStudents() { return students; }


This is a bad idea. It returns a reference to an internal class member. Think what happens if the Class instance is deleted but some external entity is still holding a reference to data that no longer exists. The only place it's used is within GradeCalculator::averageGrade() and GradeCalculator::DislayResult() so that's a strong indicator that something is not right in the class interface. I'd suggest making the averageGrade() function a Class member function.

## Use defaults when sensible

The Student constructor is essentially the same as that which would have been generated by the compiler. To reduce the chance of error, it can be simply eliminated.