4

I am having a little trouble working with nested conditional operators in C.

int is_correct() {

  char YN ;

  printf( "Y or N : " ) ;
  scanf( "%c", &YN ) ;
  YN = toupper( YN ) ;

  return ( YN == 'Y' )? 1 : ( YN == 'N' )? 0 : is_correct() ;
}

I was under the impression that the last line of code would return 1 or 0 if a 'Y' or 'N' was entered or call itself again if an unexpected character was entered. Instead it continuously calls itself no matter the input.

X_Trust
  • 657
  • 1
  • 6
  • 25
  • 2
    This may not be the issue, but I'd be using parentheses in a multi-conditional: `return (YN == 'Y') ? 1 : ((YN == 'N') ? 0 : is_correct()) ;` – lurker Jun 29 '13 at 20:13
  • 1
    Why are you writing silly code and expect a non-silly answer? – Ed Heal Jun 29 '13 at 20:14
  • 3
    @Mat: No, it isn't. As long as the conditions are wrapped in `()`, as in the OP's case, there are no ambiguities whatsoever in the parsing of these conditional operators. The extra `()` are completely unnecessary. For some they might improve the readability, for some they might actually make it worse. In fact the `()` around conditions are also unnecessary, but they do help to read the code. As for additional `()`... no, the OP's version is perfectly fine. – AnT stands with Russia Jun 29 '13 at 20:16
  • `int is_correct(void)` and parentheses. – hit Jun 29 '13 at 20:18
  • @hit: You don't need the `void` here and parens are equally unnecessary – Ed S. Jun 29 '13 at 20:21
  • 2
    @Ed S: unnecessary, sure. but it makes for a easier read. `(void)` is the correct way to indicate no parameters in C, not `()` – hit Jun 29 '13 at 20:26
  • @hit: It's readable to me. I don't require a massive amount of unnecessary parens to process that line of code. Sure, a lack of `void` means "a function which takes an unspecified number of arguments" in C but, in this case, is entirely irrelevant. – Ed S. Jun 29 '13 at 20:32
  • 1
    Why on earth do you want to call the function recursively when a simple loop would suffice? the recursion is a path only to a stack overflow if the user enters enough wrong characters. – camelccc Jun 29 '13 at 20:34
  • 1
    @EdS. having `void` in the definition has the (little) benefit of forcing the diagnostic if `is_correct` is incorrectly called with arguments. – ouah Jun 29 '13 at 20:35
  • 1
    @ouah: Sure, I'm not saying that it makes no difference in a theoretical situation... I'm just saying it makes no difference in the context of this question. – Ed S. Jun 29 '13 at 20:38
  • 1
    Funny thing is, I just compiled this silly code (using gcc 3.4) and it does behave exactly like the questioner said he thought it should, So I have to ask what compiler are you using? - compiler bugs are most likely when compiling silly code – camelccc Jun 29 '13 at 20:41
  • 3
    @camelccc: If `YN` is a `signed char` then the call to `toupper()` invokes UB. – Ed S. Jun 29 '13 at 20:45
  • Works like a charm for me. By the way: using scanf here isn't a good idea. – qwertz Jun 29 '13 at 21:20
  • @user1744194 For debugging purposes, it may help to add a `printf()` call here and there in order to verify that the contents of variables are the way you think about them. – glglgl Jul 08 '13 at 09:54

3 Answers3

2

Probably the scanning is failing, and you're not verifying it.

You don't specify if "continuously" means "without stopping to read more input", which it of course should do.

Note, for instance, that toupper() uses int-typed argument and results, and expecting values of type unsigned char you might hit undefined behavior there.

This is a really confusing aspect of the ctype.h functions. I tend to cast to unsigned char in the call, if the data comes from a text (char) buffer.

Add a printf() call to print out the value of YN before the final line.

unwind
  • 391,730
  • 64
  • 469
  • 606
  • 2
    UB comes into play if `c` is not an `unsigned char`, which is probably the case here. – Ed S. Jun 29 '13 at 20:15
  • There's nothing wrong with the `scanf()`; the problem is that the `%c` conversion is specified to behave differently from the way you might reasonably expect it to. – Adam Liss Jun 29 '13 at 21:05
1

The %c conversion does not read the newline character that you must type after your single-character answer.

In general, scanf() causes far more problems than it's worth. Try using getline() to read a string from stdin into a string, and then use sscanf() to extract a character from the string.

Among other things, getline() allows you to specify the maximum input length, so you can easily avoid overflowing a string buffer.

Adam Liss
  • 47,594
  • 12
  • 108
  • 150
  • 1
    `fgets()` was deprecated a while ago and removed in C11. You should use `fgets_s()` instead. – idoby Jun 29 '13 at 21:07
  • Thank you, @busy_wait. Replaced `fgets()` with `getline()`, as explained at http://stackoverflow.com/questions/16323185/why-is-the-fgets-function-deprecated – Adam Liss Jun 29 '13 at 21:12
  • 2
    Not true. Maybe you're thinking of gets(). – fizzer Jun 29 '13 at 21:27
  • @fizzer: You are correct, my mistake. `fgets()` is still unsafe, though. – idoby Jun 29 '13 at 21:49
  • Not sure there's anything unsafe about it in OP's scenario of interacting with the user at a terminal. – fizzer Jun 29 '13 at 21:57
  • No, this example is safe, but `scanf()` is notorious for creating string buffer overflows. The standard seems to be tending toward requiring the caller to specify a maximum length, so at least there's no guesswork in choosing how much memory to allocate. As our reliability folks are fond of saying, hope is not a strategy. :-) – Adam Liss Jun 30 '13 at 00:36
0

In scanf("%c",&YN) place a space before conversion specifier%c like scanf(" %c",&YN) to eat up newline character(\n) 

#include <stdio.h>
#include <ctype.h>

int is_correct(void) 
{
   char YN ;
   printf( "Y or N : " ) ;
   scanf(" %c",&YN);
   YN = toupper( YN );

   return  YN == 'Y' ? 1 :  YN == 'N' ? 0 : is_correct() ;
}     

int main()
{ 
    printf("%d",is_correct());
    return 0;
}

I have tested it. Working fine if you type only one character(excluding \n)!
In a more efficient way you can do like this; store first character into char ch and then use a loop while((YN = getchar()) != '\n') to eat up all other characters including \n. Ex: If you type ynabcd, first character y will be stored in ch as Y and rest will be eat up by while( loop. 

int is_correct(void) 
{
   char YN ;
   printf( "Y or N : " ) ;
   scanf("%c",&YN);
   char ch = toupper( YN );
   while((YN = getchar()) != '\n')
    ; 
   return  ch == 'Y' ? 1 :  ch == 'N' ? 0 : is_correct() ;
}
haccks
  • 104,019
  • 25
  • 176
  • 264
  • 2
    It doesn't matter whether you use `getchar` or `scanf("%c")` the issue is the same: you're only reading 1 character from the input buffer but there are two: the real char you typed and \n because you ended input with ENTER. So the function will read the character with `getchar` and sees that there is one left and checks it's value again. If you enter more than 1 character like "ab", you will get "Y or N" 3 times because there are 3 characters in the buffer. That's why you're seeing "Y or N" twice. – qwertz Jun 29 '13 at 21:26
  • But when you will use `scanf` it will not give the result on entering character other than `Y` or `N`.But your point is valid, main problem is `\n`. – haccks Jun 29 '13 at 21:31
  • To remove this issue you just have to read all characters out of the buffer before a potential re-call: putting `while (getchar() != '\n');` before the return statement would be one solution. – qwertz Jun 29 '13 at 21:39
  • @haccks btw which is you c compiler? – noufal Jun 30 '13 at 01:19