-5
#include "stdafx.h"
#include <iostream>
#include <string>

#include <stdio.h>
#include <stdlib.h>
#include <time.h>



using namespace std;

//this program will let the user input their assignment score and see their letter grade
int main() {

    int score;

    cout << "Input your score: ";

    //to make the while loop
    int x = 1;

    while (x == 1) {

        cin >> score;

        if (score >= 90){
            cout << "\nA";
            break;
        }

        else if (score >= 80) {
            cout << "\nB";
            break;
        }

        else if (score >= 70) {
            cout << "\nC";
            break;
        }

        else if (score >= 60) {
            cout << "\nD";
            break;
        }

        else if (score >= 0) {
            cout << "\nF";
            break;
        }

        else
            cout << "\nInvalid input";
    }
}

I'm trying to write a program that let the user input their score for an assignment and display their resulting letter grade. If the user input is not a valid score, it prints "Invalid input" and should ask for user input again. However, when I actually run the program and type in an invalid value, it goes into an infinite loop of printing "Invalid input". Why is this? Thanks in advance.

Banaandrew
  • 17
  • 2
  • 3
    "input your letter grade"? so people are entering `A+`? if that's the case, you're comparing the ascii value of A to your scores, e.g. A->65, B->66, and none of your score values will ever match. – Marc B May 15 '15 at 14:13
  • I don't see how will it ask for an input again after breaking out of the loop. – Eugene Sh. May 15 '15 at 14:14
  • 1
    @EugeneSh.: It won't break out of the loop if reading the score failed. In principle. – Lightness Races in Orbit May 15 '15 at 14:14
  • @LightnessRacesinOrbit Yes, but still. – Eugene Sh. May 15 '15 at 14:15
  • you maybe should edit `cout << "Input your letter grade: ";` because it looks as if the user is meant to write 'A' or 'B' or any other character. I think you want the user to put in 90 or 80 or any other int number, right? – leAthlon May 15 '15 at 14:26
  • @EugeneSh.: Still what?! – Lightness Races in Orbit May 15 '15 at 14:42
  • @LightnessRacesinOrbit The program is *supposed* to break the loop if some condition happens and then ask for input again. I have just pointed, that even if one of the conditions met, there is no way it to get back to the input part. – Eugene Sh. May 15 '15 at 14:44
  • @EugeneSh.: I think that's deliberate. The OP means to ask for valid input once, but wants to give the user as many chances as they need to indeed make the input valid. I see no evidence that the program is supposed to ask for input again after one of those `break`s. – Lightness Races in Orbit May 15 '15 at 14:53
  • Yeah my bad I meant to to input the score – Banaandrew May 15 '15 at 23:05

2 Answers2

7

When the user enters invalid input, cin >> score fails and leaves an error flag set on the stream.

Subsequent read operations just don't do anything until you clear that flag with std::basic_ios::clear().

Furthermore, since the read failed, score has some unspecified value (as you did not initialise it), and apparently on your test runs that unspecified value happens not to match any of those continues, so you never hit a break.

Instead of just:

std::cin >> score;

Try this:

if (!(cin >> score)) {
   // If reading into an int failed, we come here
   cout << "Invalid value! Try again" << endl;

   // Clear error flag
   cin.clear();

   // Restart the loop
   continue;
}

You may also need to ask the stream to eat up newlines in the input buffer. If you get the "Invalid value!" message twice, look up on SO how to do that.

Lightness Races in Orbit
  • 378,754
  • 76
  • 643
  • 1,055
0

you set x=1 and check if x==1. For the compiler it seems that it could be an infinite loop because it's unsure if a break would happen, that's why it's only a Warning and not an Error. For that behaviour you don't even need to vast the variable x , you could use while(true) too.

leAthlon
  • 734
  • 1
  • 10
  • 22
  • Are you talking of optimization? I don't think it's that stupid.. – Eugene Sh. May 15 '15 at 14:16
  • More or less yes. I'm not sure what's the big problem here, so I guess it's the warning of the compiler that it might be an infinite loop. – leAthlon May 15 '15 at 14:17
  • Infinite loop is a totally legit construct. Especially when there are some breaks inside. – Eugene Sh. May 15 '15 at 14:18
  • It's the I/O and the `break`s that make it not UB. `while (true) {}` is UB though. – Lightness Races in Orbit May 15 '15 at 14:19
  • @LightnessRacesinOrbit Is it, really? Why? Can you point to something about it? – Eugene Sh. May 15 '15 at 14:22
  • @LightnessRacesinOrbit why UB? you can use a break in `while(true)` too. I wanted to claim that there's no need to write `int x=0; while(x==0){...}` use this instead, better readable and not wasting a variable. `while(true){...}` – leAthlon May 15 '15 at 14:22
  • @EugeneSh.: http://stackoverflow.com/a/5905171/560648 – Lightness Races in Orbit May 15 '15 at 14:43
  • @LightnessRacesinOrbit Well, that's...frustrating. Especially for embedded `while(true)` driven firmwares... – Eugene Sh. May 15 '15 at 14:47
  • @EugeneSh.: Are you saying that those pieces of firmware do not perform any I/O or writes to / reads from `volatile` variables within the loop body? If so, what is the loop body doing? – Lightness Races in Orbit May 15 '15 at 14:52
  • @LightnessRacesinOrbit The whole logic can be implemented in the interrupts. And the main will only loop forever. – Eugene Sh. May 15 '15 at 14:53
  • @EugeneSh.: Okay yeah that's UB so you'd best know what your implementation is doing with it. :) Or use a `sleep` to write to a dummy `volatile` variable every 42 days; that'd be enough. :) It's worth noting that I have no idea what the rules are in C. I'm talking about C++ (per this question; in this discussion, C++11/C++14 specifically), and one doesn't often find C++ in embedded firmware. – Lightness Races in Orbit May 15 '15 at 14:54
  • @LightnessRacesinOrbit Fortunately I have never encountered this, but still.. So the workaround would be, for example, adding some dummy volatile variable read/write in the loop? EDIT: Okay, you have answered it already..Thanks, it's a useful information. – Eugene Sh. May 15 '15 at 14:58
  • @EugeneSh.: I'm eagerly anticipating a flood of downvotes for [this admittedly highly contrived question](http://stackoverflow.com/q/30262801/560648). :) – Lightness Races in Orbit May 15 '15 at 15:03