Never Ending For Loop?

[C10ranc3]

On line 40 with the begging of a for loop what I expected the compiler to do was display the error message, list the choices once more, and until a correct choice is made repeat the process. However the for loop does this part all right but even after a correct answer is chosen and acknowledged the compiler repeats the entire error message process. Please point out any mistakes that may have led to this and how to fix it.

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

int
main ()
{

  enum gunCost
  { Basic = 10, DOUBLE = 100, Strong = 500 };
  enum fileType
  { PUBLIC = 1, CLASSIFIED = 10, TOP_SECRET };
  string country;
  string spy;
  int currency = 0;
  int x;

  cout << "What is Your Last Name?\n";
  cin >> spy;
  cout << "\nPick a Country:\n";
  cout << "\nAmerica";
  cout << "\nBritain";
  cout << "\nRussia\n";
  cin>>country;
  if (country == "America")	//Rest of the Game is played here if "America" is chosen
    {
      cout << "car";
    }
  else if (country == "Britain")	//Rest of the Game is played here if "Britain" is chosen
    {
      cout << "tea";
    }
  else if (country == "Russia")	//Rest of the Game is played here if "Russia" is chosen
    {
      cout << "bear";
    }
  else				//Invalid Selection
    {
      cout << "Please make a valid selection.\n";
      for(x=0;country=="Russia"||"America"||"Britain";){
          cout << "\nPick a Country:\n";
  cout << "\nAmerica";
  cout << "\nBritain";
  cout << "\nRussia\n";
  cin>>country;
  if (country == "America")	//Rest of the Game is played here if "America" is chosen
    {
      cout << "car";
    }
  else if (country == "Britain")	//Rest of the Game is played here if "Britain" is chosen
    {
      cout << "tea";
    }
  else if (country == "Russia")	//Rest of the Game is played here if "Russia" is chosen
    {
      cout << "bear";
    }
    }
    }
  return 0;
}

[TheIdeasMan]

Hi,

Well that is not how one does a for loop. A for loop consists of 3 parts: initialization; end condition; increment.

This part does not work how you think it does:

country=="Russia"||"America"||"Britain"

it will always be true.

You have code repetition, instead put lines 25 to 38 inside a while loop, with a bool variable valid that controls the loop ending.

http://www.cplusplus.com/doc/tutorial/control/

Good Luck !!

[JLBorges]

#include <iostream>
#include <string>

enum country_t { AMERICA = 1, BRITAIN = 2, RUSSIA = 3 };

country_t pick_a_country()
{
    std::cout << "\nPick a Country:\n"
              << AMERICA << ". America\n"
              << BRITAIN << ". Britain\n"
              << RUSSIA << ". Russia\n"
              << "enter " << AMERICA << " or " << BRITAIN << " or " << RUSSIA << ": " ;

    int choice ;
    if( std::cin >> choice && ( choice == AMERICA || choice == BRITAIN || choice == RUSSIA ) )
        return country_t(choice) ; // valid choice: return associated country_t

    std::cout << "invalid input\n" ;
    std::cin.clear() ; // clear a potential failed state (user did not enter a number)
    std::cin.ignore( 1000, '\n' ) ; // throw the bad input away
    return pick_a_country() ; // try again
}

int main()
{
    const auto country = pick_a_country() ;

    if( country == AMERICA )
    {
        std::cout << "America: car\n" ;
        // ...
    }
    else if( country == BRITAIN )
    {
        std::cout << "Britain: tea\n" ;
        // ...
    }
    else
    {
        std::cout << "Russia: bear\n" ;
        // ...
    }
}

[Cheddar99]

I'm only a beginner myself, but I was tempted to try and rewrite the poster's original code in a more simple way, per below. I see from above (JLBorges) I could/should have tidied up with the input stuff much better, so I've learned something. However, I figured using a switch statement seemed cleaner than all the else/ifs. Is there any reason that wouldn't be suitable?

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

int main ()
{
  int countrycode=0;
  string spy;

  cout << "What is Your Last Name? ";
  cin >> spy;

  cout << "\nPick a Country: (enter 1,2 or 3)\n";
  cout << "1. America\n";
  cout << "2. Britain\n";
  cout << "3. Russia\n";

  while(countrycode<1 || countrycode>3)
  {
      cout << "Enter country code: ";
      cin >> countrycode;
  }

  switch(countrycode) {
  case 1: cout << "car\n";            // America
          break;
  case 2: cout << "tea\n";            // Britain
          break;
  case 3: cout << "bear\n";           // Russia
          break;
  default:
          cout << "You shouldn't get here";
  }
  return 0;
}

[JLBorges]

> However, I figured using a switch statement seemed cleaner than all the else/ifs.
> Is there any reason that wouldn't be suitable?

No. Using a switch statement would be a good idea.

[TheIdeasMan]

Hi,

Justed wanted to point out that one can use enums in the switch. This is an advantage because one can get the compiler to warn about missing cases, if one doesn't provide a case for each enum value. There are other compiler options to warn about a missing default case too.

From the gcc manual, not sure if there are similar options on Visual Studio, there might be:

gccmanual wrote:

-Wswitch

Warn whenever a switch statement has an index of enumerated type and lacks a case for one or more of the named codes of that enumeration. (The presence of a default label prevents this warning.) case labels outside the enumeration range also provoke warnings when this option is used (even if there is a default label). This warning is enabled by -Wall.
-Wswitch-default

Warn whenever a switch statement does not have a default case.
-Wswitch-enum

Warn whenever a switch statement has an index of enumerated type and lacks a case for one or more of the named codes of that enumeration. case labels outside the enumeration range also provoke warnings when this option is used. The only difference between -Wswitch and this option is that this option gives a warning about an omitted enumeration code even if there is a default label.
-Wswitch-bool

Warn whenever a switch statement has an index of boolean type and the case values are outside the range of a boolean type. It is possible to suppress this warning by casting the controlling expression to a type other than bool. For example:

switch ((int) (a == 4))
{
…
}

This warning is enabled by default for C and C++ programs.
-Wswitch-unreachable

Warn whenever a switch statement contains statements between the controlling expression and the first case label, which will never be executed. For example:

switch (cond)
{
i = 15;
…
case 5:
…
}

-Wswitch-unreachable does not warn if the statement between the controlling expression and the first case label is just a declaration:

switch (cond)
{
int i;
…
case 5:
i = 5;
…
}

This warning is enabled by default for C and C++ programs.

[C10ranc3]

Thanks for taking a look at this! I'm not that familiar with switch statements, but I'll make sure to try that and to clean up my code a bit.