Other way to write the same code

[Gantzias]

Is there a simpler way to rewrite this code and for it to have the same outcome?

while(mark!=1 && mark!=2 && mark!=3 && mark!=4 && mark!=5 && mark!=6 && mark!=7 && mark!=8 && mark!=9)
            {
                cout<<"Wrong input. Try again."<<endl;
                cin>>mark;
            }
            if(mark==0)
            {
                cout<<"";
            }
            else
            {
                square[--mark] = 'X';
            }

//I think this works aswell but is just a pain to write

while(mark!=1 && mark!=2 && mark!=3 && mark!=4 && mark!=5 && mark!=6 && mark!=7 && mark!=8 && mark!=9)
            {
                cout<<"Wrong input. Try again."<<endl;
                cin>>mark;
            }
if(mark==1 && mark==2 && mark==3 && mark==4 && mark==5 && mark==6 && mark==7 && mark==8 && mark==9)
{
                square[--mark] = 'X';
}

[integralfx]

while(mark!=1 && mark!=2 && mark!=3 && mark!=4 && mark!=5 && mark!=6 && mark!=7 && mark!=8 && mark!=9)

while(mark < 1 && mark > 9)

[Gantzias]

Then what about if the user inputs letters?

[E0p9LyTq]

#include <cctype>

while ((toupper(mark < 'A') && (toupper(mark > 'Z'))

while ((tolower(mark < 'a') && (tolower(mark > 'z'))

http://www.cplusplus.com/reference/cctype/toupper/

[mbozzi]

Well from the code you've shown here, mark is an integer.
Regardless of the fact that an integer can only contain a number, you're still doing your error checking incorrectly. In fact, there is none.

The stream contains a number of error state flags which tell you when it has failed and offer some information regarding why it has failed. If the user inputs letters, for instance, then the stream will enter a failed state which must be cleared before you use the stream again.

You should check those flags and handle any errors.
There are four: std::ios_base::goodbit, std::ios_base::failbit, std::ios_base::badbit, std::ios_base::eofbit.

You can find more information about them here:
http://en.cppreference.com/w/cpp/io/ios_base/iostate

There is also an "exception mask", which causes exceptions to be thrown when a certain error bit would be set instead. These are unset by default on all streams, but I usually set the exception mask to cause an exception to be thrown on an "irrecoverable stream error" by writing (for std::cin) std::cin.exceptions (std::ios_base::badbit);. See
http://en.cppreference.com/w/cpp/io/basic_ios/exceptions .

If you want to repeatedly prompt the user, then you can perform the read in a for loop:

int mark = 0;
while (! std::cin >> mark) {
  /* Check to see why the stream failed: */
  if (std::cin.rdstate() & std::ios_base::badbit) {
    /* Irrecoverable stream error (permissions? lost connection?) */
  } if (std::cin.rdstate () & std::ios_base::eofbit) { 
    /* There's no more input waiting for you. */ 
  } if (std::cin.rdstate () & std::ios_base::failbit) { 
    /* The extraction failed.  Probably the user input garbage (letters?). */
  }

  /* Clear the error state, then flush any garbage waiting in the stream.  
     Now you can the stream again. */
  std::cin.clear ();
  std::cin.ignore (std::numeric_limits <std::streamsize>::max ());
}

You should add error checking to your program. Don't just let your program crash, please.

[andywestken]

@FurryGuy

Your code look a bit weird -- did you forget some brackets?

As it stands, you're feeding the result of operator< and operator> to tolower()

Andy

[E0p9LyTq]

Your code look a bit weird -- did you forget some brackets?
There were brackets, somehow they went on vacation when I copied the code. Fixed, and thanks for pointing it out.

[andywestken]

Still not quite right? (Your recent edit, but I won't quote a time to avoid time zone confusion!)

while ((toupper(mark < 'A') && toupper(mark > 'Z'))

still feeds the result of oprator< (mark < 'A', which evaluates to true or false, which end up as 1 or 0 as toupper takes an int), and has unbalanced brackets.

Howabout?

while ((toupper(mark) < 'A') && (toupper(mark) > 'Z'))

or

while (toupper(mark) < 'A' && toupper(mark) > 'Z')

Andy

[E0p9LyTq]

I had it correctly formatted, and still it posted as incorrect. It took 4 tries to get it showing up as it should. One of the postings had some text replaced by gibberish.

The first error was my mistake, I admit it. The other(s) not so much.

[dhayden]

Then what about if the user inputs letters?

What about it? The point of integralfx's post is that his code is an easier way to do what you have in your while loop. If he isn't handling letters then neither is your original code. Actually his code isn't quite right. It should bewhile(mark < 1 || mark > 9)

I've always found it cleaner to break out of the middle of the loop when checking input. The pattern is basically

while (true) {
    get input
    if input is good then break
    else print message and get ready to receive input again.
}
process valid input.

Applying this to your code:

while(true) {
    cin >> mark;
    if (mark >= 1 && mark <= 9) break;
    // input is bad
    cout << "Input must be between 1 and 9\n";
}
square[--mark] = 'X';

[andywestken]

@dhayden

The code you posted will hit an infinite loop if you enter a non-numeric string, as you aren't clearing the istream's error flags. as mbozzi has mentioned.

Andy

#include <iostream>
#include <limits> // for numeric_limits<>
using namespace std;

int main()
{
    char square[9] = { 0 };

    int mark = 0;
    while (true) {
        cin >> mark;
        if (mark >= 1 && mark <= 9) break;
        // input is bad
        cout << "Input must be between 1 and 9\n";
        cin.clear(); // reset flags
        cin.ignore(numeric_limits<streamsize>::max(), '\n'); // throw away rest of line
    }
    square[--mark] = 'X';

    return 0;
}

[Gantzias]

Thanks everyone for the help!
I dont get cin.ignore(numeric_limits<streamsize>::max(), '\n'); as i never used it or seen it before... Can someone explain me that? Also i played around with some code to try and figure it out. Shouldn't this allways output Error and ask for a new number? Why doesnt it loop the whole thing?

#include <iostream>
#include <limits>

using namespace std;

int main()
{
    int x;
    cin>>x;
    while(!cin>>x)
    {
        cout<<"Error"<<endl;
        cin.clear();
        cin.ignore(numeric_limits<streamsize>::max());
        cin>>x;
    }
}

[E0p9LyTq]

cin.ignore, (std::basic_istream::ignore):
http://en.cppreference.com/w/cpp/io/basic_istream/ignore
http://www.cplusplus.com/reference/istream/istream/ignore/