findLowest() and findHighest() functions in this simple program

Question

My code does the job for the most part, but I don't think the two functions that look for the highest and lowest score are the best approach. The instructions tell me to use the int as the return instead of double, but I can't tell how to take this approach.

A particular talent competition has 5 judges, each of whom awards a score between 0 and 10 to each performer. Fractional scores are allowed. The final score of a performer is determined by dropping the highest and lowest score received, then averaging the 3 remaining scores. Should include the following functions:

• void getJudgeData() - should ask the user for the judge's score, store it in a reference parameter variable, and validate it. Called in main() for each 5 judges.

• double calcScore() - should calculate and return the average of the 3 scores that remain after dropping the highest and lowest scores the performer received. This function can only be called once by main() and should be passed the 5 scores.

Two additional functions should be called by calcScore(), which uses the returned info to determine which of the scores to drop:

• int findLowest() - should find and return the lowest of the 5 scores passed to it.

• int findHighest() - should find and return the highest of the 5 scores passed to it.

#include <iostream>
#include <iomanip>
using namespace std;

// Function prototypes
void getJudgeData(double &someValue);
double calcScore(double a, double b, double c, double d, double e);
double findLowest(double aa, double bb, double cc, double dd, double ee);
double findHighest(double aaa, double bbb, double ccc, double ddd, double eee);

int main ()
{   double scores, avgValue, judge1, judge2, judge3, judge4, judge5;

    // Ask for the score of each judge
    getJudgeData(scores);
    judge1 = scores;
    cout << judge1 << endl;
    getJudgeData(scores);
    judge2 = scores;
    cout << judge2 << endl;
    getJudgeData(scores);
    judge3 = scores;
    cout << judge3 << endl;
    getJudgeData(scores);
    judge4 = scores;
    cout << judge4 << endl;
    getJudgeData(scores);
    judge5 = scores;
    cout << judge5 << endl;

    //Outputs the average value
    avgValue = calcScore(judge1, judge2, judge3, judge4, judge5);
    cout << avgValue;

    return 0;
}


/* getJudgeData asks the user for a score between 1 and 10
   gets store in a reference value and validates the input. */
void getJudgeData(double &someValue)
{   cout << "Enter input of your score from 1-10";
    cin >> someValue;
    if (someValue < 1 || someValue > 10) 
    { cout << "Please enter a valid score from 1-10";
      cin >> someValue;
    }
    else {
      someValue;
    }
}

/* calcScore calculates and return the average of 4 scores that remain after
   dropping the highest and lowest scores of the performer revieced. */
double calcScore(double a, double b, double c, double d, double e)
{   double rtValue;
    double lowScore = findLowest(a, b, c, d, e);
    double highScore = findHighest(a, b, c, d, e);

    rtValue = ((a + b + c + d + e) - (lowScore + highScore))  / 3;

    return rtValue;
}

double findLowest (double aa, double bb, double cc, double dd, double ee)
{   
    double lowestScore;

    if ((aa < bb) && (aa < cc) && (aa < dd) && (aa < ee)) {
    lowestScore = aa;
    }
    else if ((bb < aa) && (bb < cc) && (bb < ee) && (bb < ee)) {
    lowestScore = bb;
    }
    else if ((cc < aa) && (cc < bb) && (cc < dd) && (cc < ee)) {
    lowestScore = cc;
    }
    else if ((dd < aa) && (dd < bb) && (dd < cc) && (dd < ee)) {
    lowestScore = dd;
    }
    else if ((ee < aa) && (ee < bb) && (ee < cc) && (ee < dd)) {
    lowestScore = ee;
    }
    return lowestScore;
}

double findHighest (double aaa, double bbb, double ccc, double ddd, double eee)
{   
    double highestScore;

    if ((aaa > bbb) && (aaa > ccc) && (aaa > ddd) && (aaa > eee)) {
    highestScore = aaa;
    }
    else if ((bbb > aaa) && (bbb > ccc) && (bbb > eee) && (bbb > eee)) {
    highestScore = bbb;
    }
    else if ((ccc > aaa) && (ccc > bbb) && (ccc < ddd) && (ccc > eee)) {
    highestScore = ccc;
    }
    else if ((ddd > aaa) && (ddd > bbb) && (ddd > ccc) && (ddd > eee)) {
    highestScore = ddd;
    }
    else if ((eee > aaa) && (eee > bbb) && (eee > ccc) && (eee > ddd)) {
    highestScore = eee;
    }
    return highestScore;
}

Answer 1

With C++11, some code may be simplified:

double findLowest (double aaa, double bbb, double ccc, double ddd, double eee)
{
    return std::min({aaa, bbb, ccc, ddd, eee});
}

double findHighest (double aaa, double bbb, double ccc, double ddd, double eee)
{
    return std::max({aaa, bbb, ccc, ddd, eee});
}

But if you use arrays and this algorithm, the code may be simplified even more:

/* calcScore calculates and return the average of 4 scores that remain after
   dropping the highest and lowest scores of the performer revieced. */
double calcScore(const double (&a)[5])
{
    auto it = std::minmax_element(std::begin(a), std::end(a));

    return (std::accumulate(std::begin(a), std::end(a), 0.)) - (*it.first + *it.second))  / 3;
}

int main ()
{
    double judges[5];

    // Ask for the score of each judge
    for (auto& judge : judges) {
        getJudgeData(judge);
        std::cout << judge << std::endl;
    }

    //Outputs the average value
    avgValue = calcScore(judges);
    std::cout << avgValue;

    return 0;
}

Answer 2

You have a little bug in your application. The request had been each of whom awards a score between 0 and 10 but you define a invalid score (someValue < 1 || someValue > 10). So if you take between literaly, invalid should be (someValue < 1 || someValue > 9) or if you take it between with including the invalid should be (someValue < 0 || someValue > 10)

Naming

void getJudgeData(double &someValue);
double calcScore(double a, double b, double c, double d, double e);
double findLowest(double aa, double bb, double cc, double dd, double ee);
double findHighest(double aaa, double bbb, double ccc, double ddd, double eee);  

these parameternames aren't meaningful. You should always use meaningful names for method names, method parameters and also variable names.

Answer 3

You can simplify your code greatly, and make it more general-purpose, by using an array for the scores, rather than individual variables. So change e.g.

double scores, avgValue, judge1, judge2, judge3, judge4, judge5;

to

double scores[5], avgValue;

and use a loop to read in the five scores.

You'll also want to change functions such as calcScore so that they take an array rather than separate scores, e.g.

double calcScore(double scores[], int num_scores)
{
    // ...
}

Answer 4

Your use of the input routine getJudgeData() is a bit cumbersome. You can read directly into the variable you want.

int main() {
    double judge1, judge2, judge3, judge4, judge5;

    // Ask for the score of each judge
    getJudgeData(judge1);
    std::cout << judge1 << std::endl;
    getJudgeData(judge2);
    std::cout << judge2 << std::endl;
    …
}

But that is repetitive. When you have a collection of variables that are more or less the same as each other, it is likely that an array is appropriate. (Better yet, use a std::vector<double>.)

double calcScore(double scores[], size_t n) {
    …
}

int main() {
    double scores[5];

    // Ask for the score of each judge
    for (int i = 0; i < sizeof(scores) / sizeof(scores[0]); i++) {
        getJudgeData(scores[i]);
        std::cout << scores[i] << std::endl;
    }

    // Output the average after discarding the extremes
    std::cout << calcScore(scores, sizeof(scores) / sizeof(scores[0])) << std::endl;
}

That will easily scale up to any number of judges — just change the 5 to any number.

Answer 5

The function void getJudgeData(double &someValue) has some flaws.

...
cin >> someValue;
if (someValue < 1 || someValue > 10) 
{ cout << "Please enter a valid score from 1-10";
  cin >> someValue;
}
...

One thing that pops out immediately is that this code only allows the user to make one mistake. If the user inputs two incorrect values in a row, it gets accepted regardless. To fix this you can use a while loop instead of an if statement.

The second issue I see is that if the user were to input non numerical values, then your code would crash. To fix this you should add some error checking to see if the input is indeed a number.

void getJudgeData(double &someValue)
{
    std::cout << "Enter input of your score from 0-10" << std::endl;
    while(true) 
    {
        std::string input = "";
        std::cin >> input;
        std::stringstream myStream(input);
        // myStream >> someValue will evaluate false if the str -> double conversion is
        // invalid. If it is valid, the result is stored in someValue.
        // The someValue >= 0 && someValue <= 10 checks will only be evaluated if the
        // first condition is true. Meaning if the input is invalid, you don't have to
        // worry about these checks breaking the program.
        if (myStream >> someValue && someValue >= 0 && someValue <= 10)
        {
            return;
        }
        std::cout << "Invalid number. Enter input of your score from 0-10" << std::endl;
    }
 }