Grade point average (GPA) calculator

Question

For anyone who's unfamiliar with this grading system.

I'd like a general review of my code, and I have a few questions:

1. Are plain types okay for the letter grades and credit hours, or could they be public typedefs?
2. Should operator<< need to call calculateGPA(), or should I create a private accessor and have that called? In order to maintain encapsulation, I'll need to keep operator<< as a friend.
3. Is the for-loop in calculateGPA() ideal for this purpose, or are there one or more STL functions that can do the same thing but more cleanly?

GPA.h

#ifndef GPA_H
#define GPA_H

#include <iosfwd>
#include <vector>
#include <utility>

class GPA
{
private:
    std::vector<std::pair<unsigned, unsigned> > grades;
    double calculateGPA() const;

public:
    void add(char, unsigned);
    friend std::ostream& operator<<(std::ostream&, GPA const&);
};

unsigned convertLetterGrade(char);

#endif

GPA.cpp

#include "GPA.h"
#include <ostream>
#include <stdexcept>

void GPA::add(char letterGrade, unsigned creditHours)
{
    unsigned numericalGrade = convertLetterGrade(letterGrade);
    grades.push_back(std::make_pair(numericalGrade, creditHours));
}

double GPA::calculateGPA() const
{
    unsigned sumGradePoints = 0;
    unsigned sumCreditHours = 0;

    for (auto iter = grades.cbegin(); iter != grades.cend(); ++iter)
    {
        sumGradePoints += iter->first * iter->second;
        sumCreditHours += iter->second;
    }

    double totalGPA = static_cast<double>(totalGradePoints) / sumCreditHours;
    return totalGPA;
}

unsigned convertLetterGrade(char letterGrade)
{
    switch (letterGrade)
    {
        case 'a':
        case 'A': return 4;
        case 'b':
        case 'B': return 3;
        case 'c':
        case 'C': return 2;
        case 'd':
        case 'D': return 1;
        case 'f':
        case 'F': return 0;
        default: throw std::logic_error("Invalid Letter Grade");
    }
}

std::ostream& operator<<(std::ostream& out, GPA const& obj)
{
    return out << obj.calculateGPA();
}

Main.cpp

#include "GPA.h"
#include <cstddef>
#include <iostream>

int main()
{
    std::cout << "# Classes: ";
    std::size_t numClasses;
    std::cin >> numClasses;

    GPA gpa;

    for (std::size_t i = 0; i < numClasses; ++i)
    {
        std::cout << "\n(" << i+1 << ") Letter Grade: ";
        char letterGrade;
        std::cin >> letterGrade;

        std::cout << "    Credit Hours: ";
        unsigned creditHours;
        std::cin >> creditHours;

        gpa.add(letterGrade, creditHours);
    }

    std::cout << "\n*** Total GPA: " << gpa;
}

Answer 1

Note that I am completely unfamiliar with the american(?) grade system and GPAs -- I am reviewing the coding style and approach, not whether or not the solution is correct.

General notes:

• Your GPA class is not really a class. It's actually just a function. It could just as well be implemented as double calculateGPA(std::vector<std::pair<char, unsigned>>). Beware of the golden hammer, especially when it comes to classes.

• You can use std::tolower() to halve the size of your switch.

• vector comes after utility in the alphabet :-)

• C++11 allows writing >> in nested templates.

• Your program won't compile: totalGradePoints is called sumGradePoints.

• You can probably use float instead of double.

• Make the local variables in calculateGPA() floating point numbers from the start, and drop the cast.

• obj should be called gpa or something like that.

• Write unit tests.

---

Are plain types okay for the letter grades and credit hours, or could they be public typedefs?

Either is fine. Using a typedef makes the intent of a variable a bit clearer. This added distinction between types could help the developer realizing he or she is making a mistake. However, a typedef is sadly just a type alias and does not create a new type! In other words, you do not get any type-safety benefits of doing so.

To get full type safety, you would need to create a (templated) wrapper class around the types in question. Doing so would be a nice exercise when you start to learn about templates.

---

Should operator<< need to call calculateGPA(), or should I create a private accessor and have that called? In order to maintain encapsulation, I'll need to keep operator<< as a friend.

I would cache the result of calculateGPA and provide an accessor to it. Have operator<< call that accessor. This means that your class would need to invalidate the cached value whenever grades is modified.

---

Is the for-loop in calculateGPA() ideal for this purpose, or are there one or more STL functions that can do the same thing but more cleanly?

The for loop is sufficiently short and sweet, and makes just a single pass (unlike most other solutions I can think of). You could always use std::accumulate and a lambda, but you would have to do so for both sumCreditPoints and sumGradeHours.

Am I guilty of converting you to the const& style? :-)