Student grade calculator using classes/ structs in C++

Question

I am trying to teach myself C++ and using Problem Solving in C++ by Walter Savitch. This is a program project from the book that I used to practice using classes/ structs and would like some feedback, please. I included in the notes the question as well as tried to specify my functions for others to try to see my train of thought on each.

It's a work in progress and as I said, I'm still learning so I know I can improve certain things or make this more efficient. Some things I noted for myself:

• In main, to get each percentage I passed as parameters 20 or 100 then 25 or 50 explicitly there and thought I should make use of global variables to make it more applicable for general use and if those percentages may want to be changed easier, but I did this for now because my focus is practicing using classes/ structs.
• along with that, I explicitly made the quiz array to hold 2 values (quiz[2]) and am considering making that a dynamic array by which I can ask the user for how many quiz grades there are (but I'll admit, I'm still working on practicing with pointers as well so Like I said, I made this to facilitate my ease of primarily focusing on uses of classes/ structs)
• to convert the numeric grade to a letter grade, I considered using a switch statement, but opted to use if, else if, else. I could add something in case the numeric grades are out of range when the user inputs it, but left it like this for now as I am inputting valid data when I test it myself.
• the question states to define and use a structure for the student record, but as I got going I had been using the StudentRecord class and so made the struct Student to be used within the StudentRecord class. I modified it a bit, I guess, but would appreciate alternative ways of going about this.

/*
1. Write a grading program for a class with the following grading policies: 

a. There are two quizzes, each graded on the basis of 10 points. 
b. There is one midterm exam and one final exam, each graded on the basis of 
100 points. 
c. The final exam counts for 50 percent of the grade, the midterm counts for 
25 percent, 
and the two quizzes together count for a total of 25 percent. 
(Do not forget to normalize the quiz scores. They should be converted to a 
percent before they are averaged in.) 

Any grade of 90 or more is an A, 
any grade of 80 or more (but less than 90) is a B, 
any grade of 70 or more (but less than 80) is a C, 
any grade of 60 or more (but less than 70) is a D, 
and any grade below 60 is an F. 

The program will read in the student’s scores and output the student’s 
record, 
which consists of two quiz and two exam scores 
as well as the student’s average numeric score for the entire course 
and the final letter grade. 

Define and use a structure for the student record.

*/

#include"stdafx.h"
#include<iostream>
#include<string>

using namespace std;

struct Student
{
string name;
char finalLetterGrade;
};

class StudentRecord
{
private:
    Student someStudent;

    double quiz[2], midterm, finalExam;
    double finalGrade;
    char finalLetterGrade;

public:
    void inputQuizzes();
    void inputMidtermGrade();
    void inputFinalGrade();

    double *getQuizzes();
    double getMidterm();
    double getFinalExam();

    double calcPercent(double grade, double outOfTotalPts, double 
percentOfTotal);
// general application for all scores //
double setFinalNumericGrade(double newFinalGrade);
char setFinalLetterGrade(char newFinalLetterGrade);
char calcFinalLetterGrade(double finalGrade);
};
// returns ptr to first quiz grade of array //
double *StudentRecord::getQuizzes() 
{
    double *quizPtr;
    quizPtr = quiz;

    return quizPtr;
}
double StudentRecord::getMidterm() { return midterm; }
double StudentRecord::getFinalExam() { return finalExam; }
// calculates percent of total final numeric grade //
// parameters : grade recieved (numerator), //
// max pts could received based on type of test/quiz(denominator), //
// percent of final numeric grade (25, 50) //
double StudentRecord::calcPercent(double grade, double outOfTotalPts, double 
percentOfTotal)
{
    double totalPercent = (grade / outOfTotalPts) * percentOfTotal;
    return totalPercent;
}
///////////////////////////////////////////////////////////////////

double StudentRecord::setFinalNumericGrade(double newFinalGrade) { return 
finalGrade = newFinalGrade; }

// takes final numeric grade as parameter //
// returns letter grade as char //
char StudentRecord::calcFinalLetterGrade(double finalGrade)
{
    /*
    Any grade of 90 or more is an A, 
    any grade of 80 or more (but less than 90) is a B, 
    any grade of 70 or more (but less than 80) is a C, 
    any grade of 60 or more (but less than 70) is a D, 
    and any grade below 60 is an F. 
    */

    if (finalGrade >= 90)
        return 'A';
    else if (finalGrade >= 80 && finalGrade < 90)
        return 'B';
    else if (finalGrade >= 70 && finalGrade < 80)
        return 'C';
    else if (finalGrade >= 60 && finalGrade < 70)
        return 'D';
    else
        return 'F';
}

 ///////////////////////////////////////////////////////////////////////////
char StudentRecord::setFinalLetterGrade(char newFinalLetterGrade) { return 
finalLetterGrade = newFinalLetterGrade; }

// user inputs grades //
void StudentRecord::inputQuizzes()
{
    cout << "Enter quiz grades : ";
    for (int i = 0; i < 2; i++)
    {
        cin >> quiz[i];
    }
}

void StudentRecord::inputMidtermGrade()
{
    cout << "Enter midterm grade : ";
    cin >> midterm;
}

void StudentRecord::inputFinalGrade()
{
    cout << "Enter final grade : ";
    cin >> finalExam;
}
/////////////////////////////////////////////////////////////////////////
int main()
{
    Student someStudent;

    cout << "Enter name : ";
    cin >> someStudent.name;

    StudentRecord student;

    student.inputQuizzes();
    student.inputMidtermGrade();
    student.inputFinalGrade();

    double *ptr;

    ptr = student.getQuizzes();
    for (int i = 0; i < 2; i++)
        cout << "Quiz "<< i+1 <<": "<< ptr[i] << endl;


    cout << "Midterm : " << student.getMidterm() << endl;

    cout << "Final Exam : " << student.getFinalExam() << endl;

    // calculations //

    double quizSum = 0;
    for (int i = 0; i < 2; i++)
        quizSum += ptr[i];

    double quizPercent = student.calcPercent(quizSum, 20, 25);
    double midtermPercent = student.calcPercent(student.getMidterm(), 100, 
25);
    double finalPercent = student.calcPercent(student.getFinalExam(), 100, 
50);

    double finalNumGrade = quizPercent + midtermPercent + finalPercent;

    student.setFinalNumericGrade(finalNumGrade);

    char letterGrade = student.calcFinalLetterGrade(finalNumGrade);

    student.setFinalLetterGrade(letterGrade);

    someStudent.finalLetterGrade = letterGrade;

    cout << "Name : " << someStudent.name << endl;
    cout << "Final Grade : " << someStudent.finalLetterGrade << endl;

    return 0;
}

Answer 1

Welcome to CodeReview StackExchange. I'm new here too, so I would try my best to review your code and give you an explanation about it. Take in consideration that all of this is my opinion.

First of all your code should explain itself. You do not need to put comments about everything that you do in your code. It's a good practice to comment your code. But, you should comment stuff that maybe hard to understand at first glance, that maybe for you or anyone else reading your code. Try to put yourself as an outsider. You would know what a piece of code is doing just by looking at it or if you come back in a year you would know what it does?

Secondly, you should not put comment to separate blocks of code (in your case multiple slashes). This would make your code a little but unreadable and extensive. When you read code, you want to scroll as little as possible to understand the code in question. And most of IDE and Editor now include a feature to collapse regions of code.

Third, You should use the correct C ++ Literals for your code. All of your double statement where declared like int double i = 20. You should declare them like double i = 20.0.

Fourth, Try to use the C ++ STL whenever possible. It offers the best C ++ feature and implementation of trivial stuff almost bug free. So you do not have the hassle of dealing with trivial and non-trivial problem.

I don't know If I have missed something, maybe someone else may point it out. If you have any question, just ask. I would gladly answer you with the best of my ability. This is the source code with the same functionality and some comments explaining the changes

#include <array>
#include <iostream>
#include <string>

// I don't like to use `using namespace`. It help me read the code better and see where all the methods are comming from.

// Use global constants to avoid errors.
// And if you want to change it later you change the value once throughout your code base
constexpr std::size_t QUIZZES_COUNT = 2;

/*
 * Try to consolidate all your struct. You were having a struct named Student with fields
 * that can be included in a student record. Hence the elmination.
*/

class StudentRecord {
public:
    std::string name;
    char finalLetterGrade;

private:
    // Try to use standard containers where posible
    std::array<double, QUIZZES_COUNT> quiz;
    double midterm, finalExam;
    double finalGrade;

public:
    void inputQuizzes();
    void inputMidtermGrade();
    void inputFinalGrade();

    double* getQuizzes();
    double getMidterm();
    double getFinalExam();

    // If a method doesn't access a field of the class, that method could be static.
    static double calcPercent(double grade, double outOfTotalPts, double percentOfTotal);
    static char calcFinalLetterGrade(double finalGrade);

    // Since this method are only setters there is no need for them to return the value
    void setFinalNumericGrade(double newFinalGrade);
    void setFinalLetterGrade(char newFinalLetterGrade);
};

double* StudentRecord::getQuizzes() {
    /*
        C style arrays are basically pointer to memory too. So you could have done something like this

            double data[2];
            return data;

        This would have returned a pointer of data. Without the necesity to create an extra variable
    */
    return quiz.data();
}

double StudentRecord::getMidterm() { return midterm; }
double StudentRecord::getFinalExam() { return finalExam; }

double StudentRecord::calcPercent(double grade, double outOfTotalPts, double percentOfTotal) {
    return (grade / outOfTotalPts) * percentOfTotal;
}

void StudentRecord::setFinalNumericGrade(double newFinalGrade) {
    // Since this method are only setters there is no need for them to return the value
    finalGrade = newFinalGrade;
}

// This method is static because doesn't access any of the member of StudentRecord
char StudentRecord::calcFinalLetterGrade(double finalGrade) {
    if (finalGrade >= 90.0)
        return 'A';
    // You don't need to prove `finalGrade < 90` since it would automatically be qualified for the above if clause.
    else if (finalGrade >= 80.0) 
        return 'B';
    else if (finalGrade >= 70.0)
        return 'C';
    else if (finalGrade >= 60.0)
        return 'D';
    else
        return 'F';
}

void StudentRecord::setFinalLetterGrade(char newFinalLetterGrade) {
    // Since this method are only setters there is no need for them to return the value
    finalLetterGrade = newFinalLetterGrade;
}

void StudentRecord::inputQuizzes() {
    // This method is unnecessary since they are only called once. Their code could be putted where they are called
    std::cout << "Enter quiz grades : ";
    for (int i = 0; i < QUIZZES_COUNT; i++) {
        std::cin >> quiz[i];
    }
}

void StudentRecord::inputMidtermGrade() {
    // This method is unnecessary since they are only called once. Their code could be putted where they are called
    std::cout << "Enter midterm grade : ";
    std::cin >> midterm;
}

void StudentRecord::inputFinalGrade() {
    // This methods is unnecessary since they are only called once. Their code could be putted where they are called
    std::cout << "Enter final grade : ";
    std::cin >> finalExam;
}

int main() {
    StudentRecord student;

    std::cout << "Enter name : ";
    std::cin >> student.name;

    student.inputQuizzes();
    student.inputMidtermGrade();
    student.inputFinalGrade();

    // General iostream tip: Don't use std::endl since it flush the stream buffer. That have performance impact
    // Source: Video by Jason Turner https://www.youtube.com/watch?v=GMqQOEZYVJQ

    double* ptr = student.getQuizzes();
    for (int i = 0; i < QUIZZES_COUNT; i++)
        std::cout << "Quiz " << i + 1 << ": " << ptr[i] << '\n';


    std::cout << "Midterm : " << student.getMidterm() << '\n';

    std::cout << "Final Exam : " << student.getFinalExam() << '\n';

    // calculations //

    double quizSum = 0.0;
    for (int i = 0; i < QUIZZES_COUNT; i++)
        quizSum += ptr[i];

    // All of this "random" numbers (20.0, 25.0, 50.0, 100.0) can be made into constant to make the code reader aware of their significance
    double quizPercent = StudentRecord::calcPercent(quizSum, 20.0, 25.0);
    double midtermPercent = StudentRecord::calcPercent(student.getMidterm(), 100.0, 25.0);
    double finalPercent = StudentRecord::calcPercent(student.getFinalExam(), 100.0, 50.0);

    double finalNumGrade = quizPercent + midtermPercent + finalPercent;

    student.setFinalNumericGrade(finalNumGrade);

    char letterGrade = StudentRecord::calcFinalLetterGrade(finalNumGrade);

    student.setFinalLetterGrade(letterGrade);

    std::cout << "Name : " << student.name << '\n';
    std::cout << "Final Grade : " << student.finalLetterGrade << '\n';

    return 0;
}
```cpp

Answer 2

Using 'using namespace std;' is considered bad practice, see https://stackoverflow.com/questions/1452721/why-is-using-namespace-std-considered-bad-practice. If another unit declares a function or variable that has the same name, the compiler might choose the wrong one and create difficult to debug bugs.

You could consider validating the input of calcFinalLetterGrade(), a value higher than 100 or lower than 0 should probably give some error/warning. Although you are the only one that will call this function.

Answer 3

Some shorter points:

• You don't need #include "stdafx.h". It's usually used to consolidate includes in Windows projects. That's not applicable here.
• Don't use using namespace std;. Instead refer to things in std with the std:: prefix. C++ (name resolution) is needlessly complicated and as a result you can get bitten by some strange bugs if names collide. You avoid all of this by not doing using namespace std;.
• Instead of double quiz[2], use std::array (std::array<double, 2> quizzes). This is preferable to passing around pointers for many reasons, the primary one being that the length of the array is encoded in the type. With a pointer (*double), doing quizzes[3] is a really easy way to introduce undefined behavior. This is bad. (Another benefit is if you change the size, you don't have to change all the usage iteration sites; with your existing code you'd have to change the loops in inputQuizzes(), main(), etc.) If you want to accept a variable number of quizzes, use std::vector instead.
• We usually prefix members with m_ to differentiate between local variables. Ex. finalGrade should be m_finalGrade
• Separate implementation from interface. All of the foreward declarations should be in a student.h file (that begins with #pragma once) and the implementation should be in a student.cpp file (that has #include "student.h")
• Usually we write comments // comment instead of // comment //. Also dividers like /////////// are rarely used. Your usage of them indicates that you probably want to split things up into different files.
• You have a lot of information duplicates (ex. Student and StudentRecord has a finalLetterGrade with different access methods (ex. directly by student.finalLetterGrade and through a setter by studentRecord.setFinalLetterGrade('A')). You're conflating the getter/setter pattern. You probably want to pick just one approach. But also, this information should only live in one place. Any duplication is an opportunity for things to accidentally go out of sync (Think if somewhere else you changed the final letter grade for the Student but not the StudentRecord. Now how do you know which finalLetterGrade is correct?).
• Some of your methods on StudentRecord don't really belong. For example, calcFinalLetterGrade (and calcPercent) doesn't perform an action on a specific student. It doesn't use any of the private member variables. It could just (and probably should) be a static function.
• getQuizzes() has a strange body. You can just return quiz as arrays decay to pointers (although as I argue above, relying on this is generally bad and you should be using std::array).
• Be careful with double. There are some numbers that cannot be represented with it. In particular, you shouldn't use floating point numbers to encode currency (because small imprecisions can lead to lost money). I'm dubious as to whether you could make the same case for grades. It is possible that with the right series of divisions you could produce a number that in a pure math class would be > 90 but wouldn't be representable and would instead be represented as closer to an 89. Since it's easier to use double, I'll stick with it, but keep this in mind.
• cin can fail. You also don't handle the case of an invalid grade. What if the user inputs a negative midterm grade?
• Representation is up to you, of course, but often we'll represent 0-100% as the range 0-1 (like you would in math) instead of 1-100 (I'll stick with your convention, but keep this in mind).
• Keep your indentation consistent. Indent blocks surrounded by {}. This makes code much easier to follow.

Overall, this is a good first attempt, but it could use some work. My main observation is that there is a lot of code (with lots of methods and several objects) for relatively little logic. This is often a sign that you can refactor to simplify things. Shorter, simpler code will almost always be easier to understand.

The first thing we'll try is eliminating all of the indirection and doing everything inside of main. You're already almost doing this. calcPercent and calcFinalLetterGrade is the only real logic done outside of it (sans the input). The only thing you've really pulled out is where the grades and student information are stored (encapsulated in an object, instead of local variables).

#include <array>
#include <iostream>
#include <numeric>
#include <string>

int main() {
    std::string studentName;
    std::array<double, 2> quizGrades;
    double midtermGrade;
    double finalExamGrade;

    std::cout << "Enter student name: ";
    std::cin >> studentName;

    // Read in grades
    for (size_t i = 0; i < quizGrades.size(); i++) {
        std::cout << "Enter quiz " << (i + 1) << " grade: ";
        std::cin >> quizGrades[i];
    }

    std::cout << "Enter midterm grade: ";
    std::cin >> midtermGrade;

    std::cout << "Enter final exam grade: ";
    std::cin >> finalExamGrade;

    // Compute final grade
    double finalGrade =
        25 * (std::accumulate(quizGrades.begin(), quizGrades.end(), 0) / (10 * quizGrades.size())) +
        25 * (midtermGrade / 100) +
        50 * (finalExamGrade / 100);

    char finalLetterGrade = (finalGrade >= 90) ? 'A' :
                            (finalGrade >= 80) ? 'B' :
                            (finalGrade >= 70) ? 'C' :
                            (finalGrade >= 60) ? 'D' : 'F';

    std::cout << studentName << " got an " << finalLetterGrade << std::endl;

    return 0;
}

Some things to note with this approach:

• The use of std::accumulate to sum an arbitrary length collection.
• I'm a little dubious about the ternary (because the precedence is abused a bit here), but it seems clear enough
• It should illustrate just how simple the business logic of your program is. There's no need to complicate it much beyond this.
• It doesn't handle invalid input (cin failing, negative grades). We'll get to this in a second

With the exception of the last point, I'd argue for this scenario, there isn't a good reason to complicate the code any further than the above. But it looks like you want to learn how to properly OOP-ify a problem, so we'll tackle this as an example. But first, the validation.

We note that for the first half of our program, we're repeating two actions for each variable we need to fill:

1. Print a message informing the user that data we are collecting
2. Read the grade from stdin

Ideally (2) should also reask the user if they input an invalid grade (say, a negative one).

We could simplify the logic by extracting it into a function.

double readGradeInteractive(std::string name) {
    double grade;

    do {
        std::cout << "Enter " << name << " grade: ";
        std::cin >> grade;
    } while (std::cin.fail() || grade < 0);

    return grade;
}

Note how this handles errors gracefully. It also allows grades > 100 to allow for extra credit points. We can use this function like so:

#include <array>
#include <iostream>
#include <numeric>
#include <sstream>
#include <string>

static double readGradeInteractive(std::string name) {
    double grade;

    do {
        std::cout << "Enter " << name << " grade: ";
        std::cin >> grade;
    } while (std::cin.fail() || grade < 0);

    return grade;
}

int main() {
    std::string studentName;
    std::array<double, 2> quizGrades;
    double midtermGrade;
    double finalExamGrade;

    std::cout << "Enter student name: ";
    std::cin >> studentName;

    // Read in grades
    for (size_t i = 0; i < quizGrades.size(); i++) {
        std::stringstream name;
        name << "quiz " << (i + 1);

        quizGrades[i] = readGradeInteractive(name.str());
    }

    midtermGrade = readGradeInteractive("midterm");
    finalExamGrade = readGradeInteractive("final exam");

    // Compute final grade
    double finalGrade =
        25 * (std::accumulate(quizGrades.begin(), quizGrades.end(), 0) / (10 * quizGrades.size())) +
        25 * (midtermGrade / 100) +
        50 * (finalExamGrade / 100);

    char finalLetterGrade = (finalGrade >= 90) ? 'A' :
                            (finalGrade >= 80) ? 'B' :
                            (finalGrade >= 70) ? 'C' :
                            (finalGrade >= 60) ? 'D' : 'F';

    std::cout << studentName << " got an " << finalLetterGrade << std::endl;

    return 0;
}

Now, you'll note this doesn't save us many lines in main() at first glance, but to handle errors, readGradeInteractive has to be a bit more complicated. So, this is a good refactoring.

I'd argue at this point, this is a fine solution to your problem. You don't really need to introduce OOP here. But, let's say we wanted to allow other programs to use this logic (the grade calculation) in their programs. Then, it may make sense to introduce a Student object, that encapsulates these grades and is capable of computing its final grade (as a number and letter grade). Why no StudentRecord? Think about what's going on here. A Student has grades. And an action you can perform on the student is asking it what its final grade is. There's no need for any intermediary objects. Let's start by redesigning our main to imagine what we need Student to be able to do.

#include <cstdlib>
#include <iostream>

#include "student.h"


int main() {
    auto student = Student::readInteractive();

    std::cout << student.name() << " got an " << student.finalGrade() << std::endl;

    return EXIT_SUCCESS;
}

Notice how much simpler this is! main no longer has any logic. It delegates reading the student name and grades in from stdin to a static method on Student. And it delegates computing the final (& letter) grade to the student object.

Let's start by writing student.h. All we need to do is look at the API we want Student to expose (namely static Student readInteractive(), char finalGrade(), std::string name()) and consult our original code to recall the member variables that student needs to encapsulate:

#pragma once

#include <array>
#include <string>


class Student {
public:
    static Student readInteractive();
    std::string name() const;
    char finalGrade() const;

private:
    std::string m_name;
    std::array<double, 2> m_quizGrades;
    double m_midtermGrade;
    double m_finalExamGrade;
};

Now, all that's left is to implement it in student.cpp:

#include "student.h"

#include <iostream>
#include <numeric>
#include <sstream>


static double readGradeInteractive(std::string name) {
    double grade;

    do {
        std::cout << "Enter " << name << " grade: ";
        std::cin >> grade;
    } while (std::cin.fail() || grade < 0);

    return grade;
}

Student Student::readInteractive() {
    Student student;

    std::cout << "Enter student name: ";
    std::cin >> student.m_name;

    for (size_t i = 0; i < student.m_quizGrades.size(); i++) {
        std::stringstream name;
        name << "quiz " << (i + 1);

        student.m_quizGrades[i] = readGradeInteractive(name.str());
    }

    student.m_midtermGrade = readGradeInteractive("midterm");
    student.m_finalExamGrade = readGradeInteractive("final exam");

    return student;
}

std::string Student::name() const { return m_name; }

char Student::finalGrade() const {
    double finalGrade =
        25 * (std::accumulate(m_quizGrades.begin(), m_quizGrades.end(), 0) / (10 * m_quizGrades.size())) +
        25 * (m_midtermGrade / 100) +
        50 * (m_finalExamGrade / 100);

    return (finalGrade >= 90) ? 'A' :
           (finalGrade >= 80) ? 'B' :
           (finalGrade >= 70) ? 'C' :
           (finalGrade >= 60) ? 'D' : 'F';
}

Note how we largely just copied the logic from our original program (changing the local variables to member variables where appropriate).

I'd wager this is a fairly reasonable application of OOP. You could take this one step further by examining the grade. Often, we like our type signatures to be very informative about the things they take and return. In the case of char finalGrade(), it may not immediately be clear that char is a letter grade. Additionally, the type char doesn't restrict the things that can be returned to just ABCDEF. An H could be returned. What would that mean? Additionally, there are other grade-like states like Incomplete, Withdrawn, etc. that you may want to encode here. But we wouldn't want to just use magic letters for them (lest we accidentally return a grade with them due to a bug). Another concern with the above approach is that it combines two concerns intot finalGrade: (1) computing the final grade as a number and (2) converting that numeric grade to a letter grade.

So, we could solve both of these by using an enum class to represent grades and make that enum responsible for converting from a numeric grade to a letter. I'll leave this as an exercise for you :)