**This is what my functions look like now: **
#include <iostream>
using namespace std;
int getScores(int array[])
{
cout << "Enter up to 10 scores, separated by pressing [space]\n"
<< "or [enter]. Enter -1 or a non-integer character to quit.\n";
int num = 0;
while(num < 10 && cin >> array[num] && array[num] != -1)
{
num++;
}
return num;
}
int countPerfect(int array[], int numScores)
{
int perfect = 0;
for(int count = 0; count < numScores; count++)
{
if(array[count]==100)
perfect++;
}
return perfect;
}
I'm going to continue messing with it, but that's it for now.
I'm writing a program in DevC++ that uses two functions, getScores
and countPerfect
, to allow a user to input up to 10 scores into an array. It also gives the user the option to end input early by entering -1 (in getScores
function). When I run the program and try to end it using -1, it doesn't work. It accepts -1 as input and continues the loop.
Not only that, but I'm pretty sure I did the countPerfect
function incorrectly, too. If I were to exit the loop successfully, albeit incorrectly (with one of the iterations of getScores
I tried), it would say "The -1
scores you entered include 0
perfect scores," regardless of the other scores I entered into the array.
How can I make this program work properly?
// Function prototypes
int getScores(int[]);
int countPerfect(int[],int);
int main()
{
const int size = 10;
int array[size];
int numScores;
// Explain program to user
cout << "This program will allow you to enter up to 10 scores. " << endl
<< "Then, it will report how many perfect scores were entered." << endl;
// Call a function to input the scores into array
numScores = getScores(array);
// Report results
cout << "The " << numScores << " scores you entered include "
<< countPerfect(array, numScores) << " perfect scores.\n";
return 0;
}
// Functions
// getScores will retrieve up to 10 scores from user
int getScores(int array[])
{
int index = 0;
while(array[index] != -1)
{
for(int index = 0; index < 10; index++)
{
cout << "Enter a score from 0-100 (-1 to quit): ";
cin >> array[index];
}
}
}
// countPerfect accepts array, then counts and returns the number of perfect scores
// (equal to 100)
int countPerfect(int array[], int numScores)
{
int perfect = 0;
for(int index = 0; index < numScores; index++)
{
if(index==100)
perfect++;
perfect = array[index];
}
return perfect;
}
Your getScores
function is broken for several reasons:
It does not return a value. If you got a "correct" value out of the function, it's by pure chance because the behavior is actually undefined.
Your function assumes the array contains some deterministic values. The first thing it does is check array[index] != -1
but actually that value is undefined because the array is uninitialized.
The inner loop reads up to 10 values and then loops back to test array[index] != -1
. But you never modified index
-- the inner loop hides that and uses its own. If you "fixed" that, then you'd be testing at index 10 which is reading off the end of the array. So you either have an infinite loop, or potential undefined behavior if you made a future change.
Additionally, the function is not well-behaved in cases where input fails. This can happen if the user enters something that's not a number, or if the stream is closed for some other reason (e.g. input from a pipe). It's a good idea to check the stream is still valid.
Depending on what version of the C++ standard your compiler implements, failure to read a value will either give you a zero (new behavior), or not write anything (old behavior). Both of these would be a problem.
Note that you could have caught #1 by enabling warnings in your compiler. It would then warn something like "not all control paths return a value". In GCC, use -Wall
to enable all warnings. It's even a good idea to also use -Werror
to treat all warnings as errors, so that you can't ignore them.
To address all this only needs one loop:
int getScores(int array[])
{
int count = 0;
while(count < 10 && cin >> array[count] && array[count] != -1)
{
count++;
}
return count;
}
Here, it will loop until one of the following things occur (tested in order):
To improve your function further, you shouldn't assume anything about the array size. A common approach is to supply it into the function, which means adding another parameter:
int getScores(int array[], int max_count)
{
int count = 0;
while(count < max_count && cin >> array[count] && array[count] != -1)
{
count++;
}
return count;
}
Since you're using C++, you should really consider using std::array
or std::vector
instead of C arrays. As an example, you could completely remove the limit of 10 values, and let the user enter as many scores as they want:
#include <algorithm>
#include <iostream>
#include <vector>
std::vector<int> getScores(std::istream& s)
{
std::vector<int> scores;
int score;
while(s >> score && score != -1)
{
scores.push_back(score);
}
return scores;
}
int countPerfect(const std::vector<int>& scores)
{
return std::count(scores.begin(), scores.end(), 100);
}
int main()
{
auto scores = getScores(std::cin);
int perfect = countPerfect(scores);
std::cout << "Perfect scores: " << perfect << "\n";
}