Accelerated C++, exercise 3-5: student grade calculator

Question

I've started learning C++, coming from a PHP/JS background. I've got a copy of the Accelerated C++ book and I'm working through it, doing each chapter and exercise. Having done a few, I'd like to get some feedback to make sure I'm not doing anything too wrong.

The chapter is introduced with the premise of

Imagine a course in which each student's final exam counts for 40% of the final grade, the midterm exam counts for 20%, and the average homework grade makes up the remaining 40%.

The particular exercise I'm doing then says

Write a program that will keep track of grades for several students at once. The program could keep two vectors in sync: the first should hold the students' names and the second the final grades that can be computed as input is read. For now, you should assume a fixed number of homework grades.

They suggest using two vectors for this; one of student names and one of grades, although I expanded slightly on that to take into account the different grades (midterms, finals and homework). I've also opted to calculate the grades after receiving all input in order to separate the code that gets and stores input from the code that performs calculations.

Here's a full copy of the program as it stands (110 lines). I'm looking for any feedback on style, correctness, language traps I may have accidentally fallen into, and really anything in general that you think may be of use.

#include <iostream>
#include <iomanip>
#include <string>
#include <vector>

using std::cin;
using std::cout;
using std::endl;
using std::setprecision;
using std::streamsize;
using std::string;
using std::vector;

/*****
 * This program is designed to take a list of students, then ask for their
 * midterm, final and homework grades. It then calculates the final grade
 * for each student, with the following weights:
 *   Midterm:  20%
 *   Final:    40%
 *   Homework: 40% of average grade
 */
int main()
{
  // How many homework grades each student requires
  int assignments = 3;

  // Get all student names and store them in the first vector
  vector<string> students;
  {
    string student;
    cout << "Please enter all students forenames, or an empty line when done."
         << endl
         << "Student name: ";

    while (getline(cin, student) && !student.empty()) {
      students.push_back(student);
      cout << "Student name: ";
    }

    cout << endl
         << students.size() << " students entered."
         << endl << endl;
  }

  // Ask for the midterm, final and `assignments` homework grades for students
  vector<double> midterms;
  vector<double> finals;
  vector<double> homework;
  {
    double grade;

    for (int s = 0; s < students.size(); s++) {
      // invariant: we've received grades for `s` students
      cout << "Grades for " << students[s] << endl;

      cout << " - Midterm: ";
      cin >> grade;
      midterms.push_back(grade);

      cout << " - Final: ";
      cin >> grade;
      finals.push_back(grade);

      for (int a = 0; a < assignments; a++) {
        // invariant: we've received the grade for `a` assignments
        cout << " - Assignment " << a + 1 << ": ";
        cin >> grade;
        homework.push_back(grade);
      }
      cout << endl;
    }
  }

  // Calculate and print the overall grades
  cout << "Overall Grades:" << endl;
  streamsize precision = cout.precision();
  {
    typedef vector<string>::size_type vec_size;
    double overallGrade,
           midtermGrade,
           finalGrade,
           homeworkGrades;

    for (vec_size s = 0; s < students.size(); s++) {
      midtermGrade   = midterms[s];
      finalGrade     = finals[s];
      homeworkGrades = 0;

      // Pull out the homework grades
      int start = s * assignments,
          end   = (s + 1) * assignments - 1;

      for (vec_size h = 0; h < assignments; h++) {
        // invariant: we've summed the grades for
        homeworkGrades += homework[start + h];
      }

      // Calculate the overall grade
      overallGrade = 0.2 * midtermGrade
                     + 0.4 * finalGrade
                     + 0.4 * homeworkGrades / assignments;

      cout << " - " << students[s] << ": "
           << setprecision(3) << overallGrade << setprecision(precision)
           << endl;
    }
  }

  return 0;
}

Answer 1

Overall, not bad for a beginner in my opinion. Some things to note:

• Use compiler warnings

  There are several (minor) warnings that you probably missed. One of them about an unused variable.

• endl vs \n

  I'm sure your book mentions this but endl will flush the buffer while \n won't.
  In most cases using \n suffices.

• Group logic together

  cout << "Student name: "; This is repeated and could be eliminated if you restructure the loop slightly.

  You should also declare variables as late as possible. So move the declaration of student right in front of the loop.

  string student;
  for (;;)
  {
      cout << Student name: ";
      getline(cin, student);
      if (student.empty())
      {
          break;
      }
      students.push_back(student);
  }
  

• Use range-based for loops

  You can use the new range based for loops instead of the old style loops e.g. for (const auto &s : students) { instead of for (int s = 0; s < students.size(); s++) {

  Adjust the next line accordingly: cout << "Grades for " << s << endl;

• Make constants constant

  The assignments variable is never changed and should therefore be declared as a constant.

• Use functions

  You are already scoping the code which is good but as @RichN pointed out you might as well use functions instead.

• Prefer using over typedefs

  As Scott Meyers suggests in his book Effective Modern C++, you should prefer the new using directive over typedefs. For example:

  using vec_size = vector<string>::size_type;
  

Answer 2

I see some things that may help you improve your code. I should note that although you're a beginner, I'm going to review the code and offer suggests as though you were expert in C++. It's not to be pedantic, but rather to try to show how C++ code might look in the future when you've acquired more knowledge and experience. Don't worry if you haven't seen all of these things yet. They will come in time.

Break up the code into smaller functions

The main() function is quite long and does a series of identifiable steps. Rather than having everything in one long function, it would be easier to read and maintain if each discrete step were its own function.

Eliminate "magic numbers"

This code contains some "magic numbers," that is, unnamed constants such as 0.2, 0.4, 3, etc. Generally it's better to avoid that and give such constants meaningful names. That way, if anything ever needs to be changed, you won't have to go hunting through the code for all instances of "0.4" and then trying to determine if this particular 0.4 means the weighting of the final grade or some other constant that happens to have the same value.

Don't use std::endl if '\n' will do

Using std::endl emits a \n and flushes the stream. Unless you really need the stream flushed, you can improve the performance of the code by simply emitting '\n' instead of using the potentially more computationally costly std::endl.

Don't store data you don't need

The description you quoted contains a hint to this effect when it says:

the final grades that can be computed as input is read.

There's no inherent problem with your approach, and it might actually be useful if it were required to be able to print each of the grades individually. If it's not required, the calculation can be done as each grade is read and only the final grade need be retained.

Separate input, output and calculation

To the degree practical it's usually good practice to separate input, output and calculation for programs like this. By putting them in separate functions, it isolates the particular I/O for your platform (which is likely to be unique to that platform or operating system) from the logic of the game (which does not depend on the underlying OS).

Consider using a custom object

If we consider that each student record contains a name and a final grade, we might create an object:

class StudentRecord {
    std::string name;
    double grade;
};

We can then have a single vector containing all records:

std::vector<StudentRecord> grades;

Printing each record could be done via a friend function:

friend std::ostream& operator<<(std::ostream &out, const StudentRecord &sr) {
    return out << sr.name << ': ' << std::setprecision(3) << sr.grade;
}

Using these plus an inserter function (not shown), your main could look like this:

int main() {
    std::vector<StudentRecord> grades;
    for (StudentRecord sr; std::cin >> sr; grades.push_back(sr)) 
    {}
    for (const auto &sr : grades) {
        std::cout << sr << '\n';
    }
}

Minimize the scope of using statements

Right now, all of the using statements are at file scope. That's not inherently awful, but it's usually good practice to reduce the scope of such statements where practical. In this case, you could simply move them all within main.

Use const where practical

The assignments variable never changes and could therefore be changed to const as in:

const int assignments = 3;

If you're using a C++11 compiler, you could use the more modern syntax and also declare it constexpr like this:

constexpr int assignments{3};

Omit return 0

When a C or C++ program reaches the end of main the compiler will automatically generate code to return 0, so there is no need to put return 0; explicitly at the end of main.

Note: when I make this suggestion, it's almost invariably followed by one of two kinds of comments: "I didn't know that." or "That's bad advice!" My rationale is that it's safe and useful to rely on compiler behavior explicitly supported by the standard. For C, since C99; see ISO/IEC 9899:1999 section 5.1.2.2.3:

[...] a return from the initial call to the main function is equivalent to calling the exit function with the value returned by the main function as its argument; reaching the } that terminates the main function returns a value of 0.

For C++, since the first standard in 1998; see ISO/IEC 14882:1998 section 3.6.1:

If control reaches the end of main without encountering a return statement, the effect is that of executing return 0;

All versions of both standards since then (C99 and C++98) have maintained the same idea. We rely on automatically generated member functions in C++, and few people write explicit return; statements at the end of a void function. Reasons against omitting seem to boil down to "it looks weird". If, like me, you're curious about the rationale for the change to the C standard read this question. Also note that in the early 1990s this was considered "sloppy practice" because it was undefined behavior (although widely supported) at the time.

So I advocate omitting it; others disagree (often vehemently!) In any case, if you encounter code that omits it, you'll know that it's explicitly supported by the standard and you'll know what it means.