0

What's wrong in the below code?
I am stuck in the do while loop. Am I comparing character wrong? I tried using scanf("%c", &answer); as well but same result

char answer[10];

    for (i = 0; i < wish; i++)
    {
        /* ... */
        /* ... */
    
        do
        {
            printf(" Does this item have financing options? [y/n]:");
            scanf("%s", &answer[i]);
        
            if ((answer[i] != 'y' )|| (answer[i] != 'n'))
            { 
                printf("\nERROR: Must be a lowercase 'y' or 'n'");
            }
        } while ((answer[i] != 'y') || (answer[i] != 'n'));
Improve this question
9
  • 2
    The logic is wrong. The test will always be true. Commented Oct 6, 2021 at 0:05
  • 2
    Perhaps you mean to check if answer[i] is not 'y' and (&&) not 'n'. Commented Oct 6, 2021 at 0:06
  • my apologize. y was the variable that contains 'y' and n contains 'n'. though i updated the post. but its still not working. Commented Oct 6, 2021 at 0:14
  • 1
    But you're checking if it's either not 'y' or not 'n'. Consider what happens if it is 'y': then the first condition is false, but the second part is true, and "false or true" is true. Commented Oct 6, 2021 at 1:01
  • 1
    Does this answer your question? Why is my c != 'o' || c != 'x' condition always true? Commented Oct 6, 2021 at 4:33

2 Answers 2

Reset to default
1

You are stuck in your loop because your condition for continuing the loop is always true.
If you have trouble seeing that try to name a letter which is neither different to "y" nor different to "n".
Take for example "a", it is different to both.
Take for example "n", it is not different to "n", but it IS different to "y".
Take for example "y", it is different to "n", though not different to "y".

So whatever letter comes in, the loop will continue.

To solve, use the comment by Chris and change to

((answer[i] != 'y' ) && (answer[i] != 'n'))

This way, any other letter than "n" or "y" will be either different than "n" AND different than "y" and will continue the loop, while both "y" and "n" will be different from at least one of the two and end the loop.

Once you got the condition right it might only be necessary once, i.e. only in the loop. The additional if is unneeded, at least in the shown code. You might have code which needs it, outside of what you show here.

Improve this answer
Sign up to request clarification or add additional context in comments.

5 Comments

Also worth noting that there's still room for improvement by removing unnecessary parentheses. (answer[i] != 'y' ) && (answer[i] != 'n') is equivalent to answer[i] != 'y' && answer[i] != 'n'
Also, I think you meant: Take for example "n", it is not different to "n", but it IS different to "y".
Thanks for your input. I do not discuss necssity of () here, coming from an environment in which coding rules (and by chance my own opinion) in favor of being bluntly explicit about what logic is meant, even if operator precedence makes it already clear to the compiler.
That's a fair point. My take on it is that the extra visual "noise" often detracts from the ability to comprehend programs.
That is a fair point. However, habits influence what is perceived as noise. Luckily I happen to be trained matching the rule which I have to follow. ;-)
0

Building on @Yunnosch's excellent answer, there us another way of looking at the logic of checking to see if something is not a set of characters.

We can look at it as checking to see if a character is not 'y' and not 'n':

answer[i] != 'y' && answer[i] != 'n'

Or... we can check to see if it is either 'y' or 'n':

answer[i] == 'y' || answer[i] == 'n'

Then simply negate that!

!(answer[i] == 'y' || answer[i] == 'n')

Another alternative

One more way of looking at this, because char is a numeric type, would be to use a switch.

switch (answer[i]) {
    case 'y':
    case 'n':
    /* Do whatever happens when there is a valid input */
    break;
   
    default:
    printf("\nERROR: Must be a lowercase 'y' or 'n'");
}

The only real advantage of using switch here is that it makes checking for things like capital y and n very easy:

switch (answer[i]) {
    case 'y': case 'Y':
    case 'n': case 'N':
    /* Do whatever happens when there is a valid input */
    break;
   
    default:
    printf("\nERROR: Must be a lowercase 'y' or 'n'");
}
Improve this answer

1 Comment

Glad to hear it! When you feel your question has been answered, it is good form on SO to accept any answers you found helpful and you may wish to vote them up.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.