Skip to main content
added 62 characters in body
Source Link
Cody Gray
  • 4.6k
  • 19
  • 30

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

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

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

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.

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;
Forgot something
Source Link
yuri
  • 4.5k
  • 3
  • 18
  • 40

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.

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.

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.

Source Link
yuri
  • 4.5k
  • 3
  • 18
  • 40

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.