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'));
phuclv
  • 37,963
  • 15
  • 156
  • 475
user3445515
  • 13
  • 3
  • 2
    The logic is wrong. The test will always be true. – Nathan Hughes Oct 06 '21 at 00:05
  • 2
    Perhaps you mean to check if `answer[i]` is not `'y'` _and_ (`&&`) not `'n'`. – Chris Oct 06 '21 at 00:06
  • my apologize. y was the variable that contains 'y' and n contains 'n'. though i updated the post. but its still not working. – user3445515 Oct 06 '21 at 00:14
  • and response can be either 'y' OR 'n'. thats why i used OR operator – user3445515 Oct 06 '21 at 00:17
  • 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. – Chris Oct 06 '21 at 01:01
  • I discussed a very similar problem here https://stackoverflow.com/a/63150087/7733418 I won't propose it as a duplicate, because I think the negation might make it non-obvious. But maybe others see it as a duplicate. – Yunnosch Oct 06 '21 at 01:14
  • You are scanning a string (multiple chars) but answer[i] is a single char. Did you mean %c? – stark Oct 06 '21 at 01:54
  • 1
    Does this answer your question? [Why is my c != 'o' || c != 'x' condition always true?](https://stackoverflow.com/questions/36605454/why-is-my-c-o-c-x-condition-always-true) – phuclv Oct 06 '21 at 04:33
  • duplicates: [Why non-equality check of one variable against many values always returns true?](https://stackoverflow.com/q/26337003/995714), [Why does || operator work in my situation](https://stackoverflow.com/q/67396397/995714) – phuclv Oct 06 '21 at 04:35

2 Answers2

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.

Yunnosch
  • 26,130
  • 9
  • 42
  • 54
  • 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'` – Chris Oct 06 '21 at 01:02
  • Also, I think you meant: Take for example "n", it is not different to "n", but it IS different to "y". – Chris Oct 06 '21 at 01:03
  • 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. – Yunnosch Oct 06 '21 at 01:06
  • That's a fair point. My take on it is that the extra visual "noise" often detracts from the ability to comprehend programs. – Chris Oct 06 '21 at 01:09
  • 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. ;-) – Yunnosch Oct 06 '21 at 01:11
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'");
}
Chris
  • 26,361
  • 5
  • 21
  • 42
  • 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. – Chris Oct 07 '21 at 02:20