tags:

views:

82

answers:

4

The following code snippets are from a c program.

The user enters Y or N.

 char *answer = '\0';

 scanf (" %c", answer);

 if (*answer == ('Y' || 'y'))
      //do work

I cant figure out why this if statement doesn't evaluate to true.

I checked for the y or n input with a printf and it is there so I know im getting the user input. Also when I replace the the condition of the if statement with 1 (making it true) it evaluates properly.

any ideas?

thanks

+7  A: 

answer shouldn't be a pointer, the intent is obviously to hold a character. scanf takes the address of this character, so it should be called as

char answer;
scanf(" %c", &answer);

Next, your "or" statement is formed incorrectly.

if (answer == 'Y' || answer == 'y')

What you wrote originally asks to compare answer with the result of 'Y' || 'y', which I'm guessing isn't quite what you wanted to do.

Mark E
I changed it but for some reason the body of the if statement still isnt evaluating
Joe Scho
@Joe, had a slight typo with an extra paren, if you copied and pasted from my answer it'd likely have failed.
Mark E
yeah i caught the typo, thanks
Joe Scho
+2  A: 

Because comparison doesn't work that way. 'Y' || 'y' is a logical-or operator; it returns 1 (true) if either of its arguments is true. Since 'Y' and 'y' are both true, you're comparing *answer with 1.

What you want is if(*answer == 'Y' || *answer == 'y') or perhaps:

switch (*answer) {
  case 'Y':
  case 'y':
    ...
    break;
  default:
    "else" code here
}
hobbs
+4  A: 

I see two problems:

The pointer answer is a null pointer and you are trying to dereference it in scanf, this leads to undefined behavior.

You don't need a char pointer here. You can just use a char variable as:

char answer;
scanf(" %c",&answer);

Next to see if the read character is 'y' or 'Y' you should do:

if( answer == 'y' || answer == 'Y') {
  // user entered y or Y.
}

If you really need to use a char pointer you can do something like:

char var;
char *answer = &var; // make answer point to char variable var.
scanf (" %c", answer);
if( *answer == 'y' || *answer == 'Y') {
codaddict
.. or by calling `malloc()`
ArunSaha
@ArunSaha: yeah, or make it point to a local char variable.
codaddict
+3  A: 

For a start, your answer variable should be of type char, not char*.

As for the if statement:

if (answer == ('Y' || 'y'))

This is first evaluating 'Y' || 'y' which, in Boolean logic (and for ASCII) is true since both of them are "true" (non-zero). In other words, you'd only get the if statement to fire if you'd somehow entered CTRLA (again, for ASCII, and where a true values equates to 1)*a.

You could use the more correct:

if ((answer == 'Y') || (answer == 'y'))

but you really should be using:

if (toupper(answer) == 'Y')

since that's the more portable way to achieve the same end.


*a You may be wondering why I'm putting in all sorts of conditionals for my statements. While the vast majority of C implementations use ASCII and certain known values, it's not necessarily mandated by the ISO standards. I know for a fact that at least one compiler still uses EBCDIC so I don't like making unwarranted assumptions.

paxdiablo