Incorrect logic flow? function that gets coordinates for a sudoku game

Question

This function of mine keeps on failing an autograder, I am trying to figure out if there is a problem with its logic flow? Any thoughts?

Basically, if the row is wrong, "invalid row" should be printed, and clearInput(); called, and return false. When y is wrong, "invalid column" printed, and clearInput(); called and return false.

When both are wrong, only "invalid row" is to be printed (and still clearInput and return false.

Obviously when row and y are correct, print no error and return true.

My function gets through most of the test cases, but fails towards the end, I'm a little lost as to why.

bool getCoords(int & x, int & y)
{
    char row;
    bool noError=true;

    cin>>row>>y;
    row=toupper(row);

    if(row>='A' && row<='I' && isalpha(row)  && y>=1 && y<=9)
    {   
        x=row-'A';
        y=y-1;  
        return true;
    }

    else if(!(row>='A' && row<='I'))
    {
        cout<<"Invalid row"<<endl;
        noError=false;
        clearInput();
        return false;   
    }

    else
    {
       if(noError)  
       {
           cout<<"Invalid column"<<endl;
       }
       clearInput();
       return false;
    }
}

Answer 1

It's hard to know without seeing the input, but here's a couple of possible issues:

(1) The way you detect that the column read failed is by examining the value of y - but are you sure that it's set to a value outside the range 1-9 by the calling code? Otherwise even if the read fails you might think it succeeds. You could fix this by adding a y=0; to the beginning of the function.

Some more explanation: when you use cin >> y to read an integer into y but there isn't an integer to read (could be because there's a non-digit in the stream, or EOF is reached, or whatever) the value of y won't be touched. So, imagine your input looks like "CB". Then after you cin >> row >> y, row contains 'C' and y still contains whatever value it had before the function was called (remember y is a reference to a variable outside this function!) This is the problem: you check to see if the read succeeded by looking at the value of y, but it's quite possible that it is between 1 and 9 even if the read failed (especially if you're calling this function in a loop and reusing the same y). So your code thinks everything went OK, even though it didn't and the stream has now got its failbit set... it's not surprising if everything goes a bit crazy after this point.

(2) The way you're reading data using extraction from cin could misbehave if a line is incomplete. That's because the reading of an integer will skip leading whitespace - including newlines. So if the (invalid) input line is just "B", for example, you'd read the "B" correctly by extracting a char, and then the integer extraction would swallow the newline and fail (assuming the next input line doesn't start with an integer). Then your ignore would completely skip the next line (unless you don't pick up on the failure because of point (1)!).