SAT score reader program

Question

This program takes 10 Math SAT scores and 10 Verbal Sat scores from the user, computes the standard deviation and averages, then displays them in a tabular format. After, it displays them to the user and is sent to a text file. I haven't done input mismatch handling in quite a while so I used the methods that you see down below.

#include <cstdlib>
#include <iostream>
#include <iomanip> 
#include <string>
#include <fstream>
#include <cmath>
#include <cctype>

using namespace std;

/**
 * 
 * @return 0
 */
int main() 
{
    int sat[10][2], mathAvg, verbAvg;
    double mathStd, verbStd;

    void describe_program();
    void read_scores(int sat[10][2]);
    void compute_means(int sat[10][2], int& mathAvg, int& verbAvg);
    void compute_std(int sat[10][2],int mathAvg, int verbAvg, double& mathStd, double& verbStd) ;
    void show_results(int sat[10][2], int mathAvg, int verbAvg, double mathStd,  double verbstd);
    bool again();

    describe_program();

    do
    {
    read_scores(sat);
    compute_means(sat, mathAvg, verbAvg);
    compute_std(sat, mathAvg, verbAvg,  mathStd, verbStd);
    show_results(sat, mathAvg, verbAvg, mathStd, verbStd);
    }while(again());
    return 0;
}

/**
 * 
 */
void describe_program()
{
    cout<<"This program reads SAT scores from 10 students (Math and verbal scores)"
        <<" and allocates the scores as elements inside a two dimensional array."
        <<endl<<"The program will then calculate the mean Math SAT score, the mean "
        <<"Verbal SAT score, and the standard deviation for both subjects."<<endl<<
        "The results will be sent to the user in a tabular format and will then "
        <<"be sent to text file."<<endl; 
}

/**
 * 
 * @param sat
 */
void read_scores(int sat[10][2])
{
    for(int i = 0; i<10;i++)
    {
        for(int j = 0; j<2; j++)
        {
            if(j == 0){
                cout<<"Enter Math score below:"<<endl;
            }

            else{
                cout<<"Enter Verbal Score below:"<<endl;
            }    


            cin>>sat[i][j];  
        }
    }
}

/**
 * 
 * @param sat
 * @param mathAvg
 * @param verbAvg
 */
void compute_means(int sat[10][2], int& mathAvg, int& verbAvg)
{
    int mathScoreSum =0;
    int mathCount = 0;
    int verbScoreSum = 0;
    int verbCount = 0;

    for(int i = 0; i<10; i++)    
    {
        for(int j = 0; j<2; j++)
        {
            if(j==0){
                mathScoreSum += sat[i][j];
                mathCount++;
            }

            else{
                verbScoreSum += sat[i][j];
                verbCount++;
            }
        }
    }
    mathAvg = mathScoreSum / mathCount;
    verbAvg = verbScoreSum / verbCount;
}

/**
 * 
 * @param sat
 * @param mathAvg
 * @param verbAvg
 * @param mathStd
 * @param verbStd
 */
void compute_std(int sat[10][2],int mathAvg, int verbAvg, double& mathStd, double& verbStd) 
{
    double mathVariance = 0;
    double verbVariance = 0;

    for(int i = 0; i<10; i++)    
    {
        for(int j = 0; j<2; j++)
        {
            if(j==0){
                mathVariance += ::pow((sat[i][j] - mathAvg), 2);
            }

            else{
                verbVariance += ::pow((sat[i][j] - mathAvg), 2);
            }

            mathStd = sqrt(mathVariance/9);
            verbStd = sqrt(verbVariance/9);
        }
    }
}

/**
 * 
 * @param sat
 * @param mathAvg
 * @param verbAvg
 * @param mathStd
 * @param verbStd
 */
void show_results(int sat[10][2], int mathAvg, int verbAvg, double mathStd, double verbStd)
{ 
    cout<<fixed<<showpoint<<setprecision(1);
    cout<<setw(13)<<"MATH"<<setw(13)<<"VERBAL"<<endl;
    for(int i = 0; i<10;i++)
    {
        for(int j = 0; j<2;j++)
        {
            if(j==0){
                cout<<setw(13)<<sat[i][j];
            }else{
                cout<<setw(13)<<sat[i][j]<<endl;
            }
        }
    }
    cout<<"MEAN"<<setw(9)<<mathAvg<<setw(13)<<verbAvg<<endl;
    cout<<"STD"<<setw(10)<<mathStd<<setw(13)<<verbStd;

    ofstream outs;
    outs.open("scores.txt",ios::app);

    outs<<fixed<<showpoint<<setprecision(1);
    outs<<setw(13)<<"MATH"<<setw(13)<<"VERBAL"<<endl;
    for(int i = 0; i<10;i++)
    {
        for(int j = 0; j<2;j++)
        {
            if(j==0){
                outs<<setw(13)<<sat[i][j];
            }else{
                outs<<setw(13)<<sat[i][j]<<endl;
            }
        }
    }
    outs<<"MEAN"<<setw(9)<<mathAvg<<setw(13)<<verbAvg<<endl;
    outs<<"STD"<<setw(10)<<mathStd<<setw(13)<<verbStd<<endl;
    outs.close(); 
}

/**
 * 
 * @return Bool variable
 */
bool again()
{
    char response;

    cout<<endl<<"Do you wish to run this program again (Y or N)?"<<endl;
    cin>>response;     

    while((response!= 'N') && (response!= 'Y') && (response!= 'y') &&
          (response!= 'n')){
        cout<<"Please enter a valid response";
        cin>>response;
    }

    response = toupper(response);

    if(response == 'Y')
        return true;
    else
        return false;    
}

Answer 1

• using namespace std; is a bad practice

• Repetitions inside the for(int j = 0; j<2; j++) loops violate the DRY principle. You'd be in a better shape computing means and averages independently for each score (to add Phys SAT you'd need to edit each function). Consider refactoring along the lines of

  int compute_mean(int sat[10]);
  double compute_std(int sat[10], int mean);
  ....
  

  calling them for each subject.

• mean should be double. Rounding mean to an integer seriously biases a deviation. You may round it when printing.

• The

  if(response == 'Y')
      return true;
  else
      return false;    
  

  is a long way to say

  return response == 'Y';
  

Answer 2

Here are a few things that I see:

Function Declarations:

void describe_program();
void read_scores(int sat[10][2]);
void compute_means(int sat[10][2], int& mathAvg, int& verbAvg);
void compute_std(int sat[10][2],int mathAvg, int verbAvg, double& mathStd, double& verbStd) ;
void show_results(int sat[10][2], int mathAvg, int verbAvg, double mathStd,  double verbstd);
bool again();

Why do you declare your functions inside main? Function protypes are usually kept outside main, since they interfere less with the logic of your code.

---

Raw Multidimensional Arrays

Using raw multidimensional arrays is bad, since they easily decay to pointers and don't know their own size. Consider implementing your own matrix class, or going with a nested std::array/std::vector.

The other thing I question is why you are even using a multidimensional array. The two series of data are not holding the same score, so I would opt for two different std::vectors for each set of scores. This would allow you to simplify your functions a bit, although you might have to call them twice.

---

Use of endl

I notice that you add endl whenever you want a newline. Beware though, this also flushes the output with it, which could have a toll on performance. Consider switching to \n.

---

::pow

Another reason why using namespace std; is considered bad practice.

---

Refactoring show_results()

Looking at your function show_results:

void show_results(int sat[10][2], int mathAvg, int verbAvg, double mathStd, double verbStd)
{ 
    cout<<fixed<<showpoint<<setprecision(1);
    cout<<setw(13)<<"MATH"<<setw(13)<<"VERBAL"<<endl;
    for(int i = 0; i<10;i++)
    {
        for(int j = 0; j<2;j++)
        {
            if(j==0){
                cout<<setw(13)<<sat[i][j];
            }else{
                cout<<setw(13)<<sat[i][j]<<endl;
            }
        }
    }
    cout<<"MEAN"<<setw(9)<<mathAvg<<setw(13)<<verbAvg<<endl;
    cout<<"STD"<<setw(10)<<mathStd<<setw(13)<<verbStd;

    // ... Basically the same thing for a file
}

It is immediately noticeable that you are repeating yourself for output to the console and output to the file. Instead, I would change the function so that it takes a std::ostream as a parameter and sends all output to this stream. Then, in main, you can call show_results() twice, once for standard output, and once for a handle to a file. Here is how I would implement this change:

void show_results(const ostream& out, int sat[10][2], int mathAvg, int verbAvg, double mathStd, double verbStd)
{ 
    out<<fixed<<showpoint<<setprecision(1);
    out<<setw(13)<<"MATH"<<setw(13)<<"VERBAL"<<endl;
    for(int i = 0; i<10;i++)
    {
        for(int j = 0; j<2;j++)
        {
            if(j==0){
                out<<setw(13)<<sat[i][j];
            }else{
                out<<setw(13)<<sat[i][j]<<endl; // Make sure to replace your endls
            }
        }
    }
    out<<"MEAN"<<setw(9)<<mathAvg<<setw(13)<<verbAvg<<endl;
    out<<"STD"<<setw(10)<<mathStd<<setw(13)<<verbStd;
}

---

Refactored Conditional

if(response == 'Y')
    return true;
else
    return false;

Can be replaced with:

return response == 'Y';

Answer 3

You can also reduce the lengthiness of computing averages using lambdas, like with the below vector:

std::vector<int> v = { 1, 2, 3, 4, 2314, 6, 7, 8, 9, 10 };
auto lambda = [&](double a, double b){return a + b / v.size(); };
double avg = std::accumulate(v.begin(), v.end(), 0.0, lambda);