Not sure if this is right

[soccer629]

Hey everyone I'm having a problem with my code. The problem I'm having is in line 57-68. The problem is to have a value that is not equal to a Y or N and when you put a different value than Y or N its an infinite loop and the other part is my count variable won't go up by 1 and end up at 3 if the user keeps putting a wrong answer, and it won't go into the if decision. Here is the whole story:


"The valid range for a test score is 0 through 100. When a test score, which is outside this range, is entered, the user is to be asked to re-enter the score. The program is to continue asking for the re-entry of a score, until a valid score is entered.In addition, the user is to enter a Y or N (lower-case should also be accepted) when asked if there are more test scores to be entered. If something other than a Y or N is entered, the user is to be prompted for a correct response. When the user is asked whether or not there are more test scores (a Y or N is to be entered), he or she is given three chances to enter a valid response. If Y or N is not entered by the third try, the program is to assume that there are no more test scores to be entered."

source code:


#include <iostream>
#include <conio.h>
using namespace std;
float GetTestScore();
float ComputeAverage(int, float);
bool MoreScores();
int main()
{
float Sum = 0.0f;
int ScoreCount = 0;

do
{
Sum += GetTestScore();
ScoreCount++;

} while (MoreScores());

float Average = ComputeAverage(ScoreCount, Sum);

cout << "\n\nAverage score: " << Average << endl;
getch();

return 0;
}
float GetTestScore() {
float score;
cout << "Enter next score: ";
cin >> score;
while(score < 0 || score > 100){
cout << "***Invalid test score. ";
cout << "\nRe-enter test score: ";
cin >> score;
}
return score;
MoreScores();
}
float ComputeAverage(int ScoreCount, float Sum){
float Average = Sum / ScoreCount;
return Average;
main();
}
bool MoreScores(){
char UserResponse;
cout << "Are there more scores to be processed? ";
cout << "\nEnter Y or N: ";
cin >> UserResponse;
if(toupper(UserResponse) == 'Y'){
return true;
GetTestScore();
}
if(toupper(UserResponse) == 'N'){
return false;
main();
}
int count = 0;
while(toupper(UserResponse) != 'Y' || toupper(UserResponse) != 'N' && count <= 3){
cout << "***Invalid Response. ";
cout << "\nMust be Y or N ---> ";
cin >> UserResponse;
count++;
}
if(count == 3){
cout << "****WARNING: Three invalid entry attempts. ";
cout << "\nNo more scores assumed. ";
return false;
main();
}
}


Any help will be appreciated. Thanks

[ak555]

while((toupper(UserResponse) != 'Y' || toupper(UserResponse) != 'N') && count <= 2)
you missed parentheses and count<=2 not <=3 as it is already entering the then block
and then incrementing count

[soccer629]

I tried that but when I first enter a "g" in for UserResponse I get my error message which is correct and then when I enter a "Y" I still get my error message and goes into an infinite loop and my count variable still doesn't work quite right.

[eker676]

You are forgetting something. After a return call is made the stuff after it is not executed but, the control is returned to the calling function.

You made this mistake in a few places.

[helios]

main();
This is the kind of thing that makes you fail.

[soccer629]

I still don't get what I'm doing wrong.

[Duthomhas]

Please use [code] [/code] tags.

#include <iostream>
//#include <conio.h>   Non-standard. Please avoid this header.
#include <limits>
#include <cctype>   // For toupper(), used below.
                    // Don't forget to include all the proper headers!

using namespace std;

float GetTestScore();
float ComputeAverage(int, float);
bool  MoreScores();

int main()
  {
  float Sum        = 0.0f;
  int   ScoreCount = 0;

  do
    {
    Sum += GetTestScore();
    ScoreCount++;
    }
  while (MoreScores());

  float Average = ComputeAverage(ScoreCount, Sum);

  cout << "\n\nAverage score: " << Average << endl;

  //getch();
  //  This is a non-standard function.
  //  The better way is as follows:
  cout << "\nPress ENTER to continue..." << flush;
  cin.sync();
  cin.ignore( numeric_limits <streamsize> ::max(), '\n' );

  return 0;
  }

float GetTestScore()
  {
  float score;
  cout << "Enter next score: ";
  cin >> score;
  while (score < 0 || score > 100)
    {
    cout << "***Invalid test score. ";
    cout << "\nRe-enter test score: ";
    cin >> score;
    }

  return score;
  //  This is an unconditional return, meaning anything that follows
  //  is meaningless. Your function should have ended here.

  //MoreScores();
  //  The function has accomplished its purpose: it asked for and validated a
  //  number from the user (a "score"), which it then returned. (See line 20,
  //  where the return value is added to 'Sum' in the main() function.)
  //  Hence, there is no need to call the MoreScores() function from here.
  }

float ComputeAverage(int ScoreCount, float Sum)
  {
  float Average = Sum / ScoreCount;
  return Average;

  //main();
  //  Please, never, ever do this. It is wrong, evil, and immoral.
  //  (Don't take that the wrong way -- just some strong language to imprint it
  //  in your brain.)
  //
  //  Again, the function accomplished its purpose and unconditionally returned
  //  a value to the main function (line 25). There is no need to execute another
  //  instance of main() -- which would be equivalent to starting the program
  //  again, except that it is illegal and/or undefined by the language standards.
  }

bool MoreScores()
  {
  char UserResponse;
  cout << "Are there more scores to be processed? ";
  cout << "\nEnter Y or N: ";
  cin >> UserResponse;

#if 1
  if(toupper(UserResponse) == 'Y')
    {
    return true;
    //GetTestScore();
    }
  if(toupper(UserResponse) == 'N')
    {
    return false;
    //main();
    }
  int count = 0;
  //while(toupper(UserResponse) != 'Y' || toupper(UserResponse) != 'N' && count <= 3)
  //  This line is what ak555 was complaining about.
  //  The && operator has a higher precedence than does the || operator.
  //  As a rule of thumb, I always add those extra parentheses to enforce
  //  proper evaluation order. In your case, you wanted:
  while ((toupper(UserResponse) != 'Y' || toupper(UserResponse) != 'N') && count <= 3)
    {
    cout << "***Invalid Response. ";
    cout << "\nMust be Y or N ---> ";
    cin >> UserResponse;
    count++;
    }
  if(count == 3)
    {
    cout << "****WARNING: Three invalid entry attempts. ";
    cout << "\nNo more scores assumed. ";
    return false;
    //main();
    }

  // Here, the function terminates without returning anything.
  // You must return something if your function is declared with a
  // non-void type.

#else
  // I think this function needs some rethinking. I suggest you use a switch
  // statement in a loop. Here is an improved version. To use it, either change the
  //   #if 1
  // line to read
  //   #if 0
  // or just delete everything above and remove the #if #else #endif lines.

  int count = 0;
  do
    {
    switch (toupper(UserResponse))
      {
      case 'Y': return true;
      case 'N': return false;
      default:
        count++;
        if (count < 3)
          {
          cout << "***Invalid Response. ";
          cout << "\nMust be Y or N ---> ";
          cin >> UserResponse;
          }
      }
    }
  while (count < 3);
        
  cout << "***WARNING: Three invalid entry attempts. "
          "\nI assume that there aren't any more scores. ";
  return false;
#endif
  }

Please remember to use proper indentation to help you keep your code straight. It does not generally matter what 'style' you use, just as long as you are consistent.

Also, when compiling, make sure to turn on all compiler warnings and ask for strict standards compliance. With the GCC, use something like:

g++ -Wall -pedantic foo.cpp


Hope this helps.

[soccer629]

Hey thanks for the help. Everything worked great.