Using boolean flag to detect input error, C

Question

I am attempting to create a boolean flag that will output an error message when an invalid input code is entered. This is for a class assignment and I am just learning C so please forgive the formatting and notify me if it could be improved. I have the code doing everything successful except for outputting the error message if the input or output variables are incorrect. I believe this is an error with the boolean check part of the code. A diagnosis would be great.

This code will:

request and source currency, destination currency, and source amount
output the destination currency amount or error message if the input variables are incorrect

Parameters:

the source and destination currency must be Canadian dollars ($), Yen (Y), or Euros (E)
a boolean flag must be used

Thanks in advanced.

# include <stdio.h>

int main() {

    char $ = $; 
    char Y = Y;
    char E = E;

    char in_currency, out_currency;

    char flag = 1;
    char new_line;

    float in_value;
    float out_value;

    //Calculation variables

    printf("Enter the source currency: ");
    scanf("%c%c", &in_currency, &new_line);

    printf("Enter the destination currency: ");
    scanf("%c%c", &out_currency, &new_line);

    if ( ( in_currency || out_currency ) != ( '$' || 'Y' || 'E' ) )
        flag = 0;

    printf("Enter the value: ");
    scanf("%f%c", &in_value, &new_line);

    //Calculations

    if (flag) {
        printf("%c%.2f = %c%.2f \n", in_currency, in_value, out_currency, out_value);
    }
    else {
        printf("There was an error with the input. \n");
    }

    return 0;
}

in_currency || out_currency ) != ( '$' || 'Y' || 'E' ) you cant do the check jn that way and p.s your char $ char E char Y are useless — Marco Acierno, Jul 13 '14 at 01:37
`$` isn't a valid variable name in C. A variable name has to start with a letter or underscore, and may only contain letters, numbers, or underscores. — Barmar, Jul 13 '14 at 02:08
These statements: `char $ = $; char Y = Y; char E = E;` invoke undefined behaviour (or, at the least, achieve nothing useful) by initializing the variable with their own uninitialized values. The variable `$` isn't valid in standard C; you can probably find a C compiler that allows it (especially if you work on VMS). That, sadly, isn't a good start. The fact that the rest of the code doesn't use the variables is an additional problem. You can simply delete them. — Jonathan Leffler, Jul 13 '14 at 02:21
Note that if the user enters 'source currency' as GPB, the `new_line` variable isn't going to contain a newline. You'd probably do better with using `" %c"` (space, percent, c) as the format string; it'll skip leading white space (blanks, tabs, newlines), and then one character. Repeat for the destination currency; it will skip the newline you expect and then read the next non-white space character. There's very little point in the `"%f%c"` format; it doesn't guarantee that you'll read a newline (and you don't check that it does). — Jonathan Leffler, Jul 13 '14 at 02:25
@Barmar it's a GCC and MSVC's extension http://stackoverflow.com/questions/7926394/in-variable-name — phuclv, Jul 15 '14 at 07:21

John Bargman · Answer 1 · 2014-07-18T12:18:42.843

Thank's to @JonathanLeffler 's great comments.

I believe this may be due to the logical order of your if statement

 if ( ( in_currency || out_currency ) != ( '$' || 'Y' || 'E' ) )
    flag = 0;

What you intended to do was as follows

 if ( 
  (in_currency == '$' ||  in_currency == 'Y' ||  in_currency == 'E')
  || 
  ( out_currency == '$' ||  out_currency == 'Y' ||  out_currency == 'E') 
  ) flag = 0;

This is due to how your original line is being evaluated. ( in_currency || out_currency )

Note the C++ standard (section 6.8.4.1) states

the substatement is executed if the expression compares unequal to 0.

is the same as typing ( in_currency != 0 || out_currency =! 0) Note that testing a single value will always test against non-zero, sometimes considered the 'true' value.

In short, the first part of the if statement will always return true, as both tests (being non-zero character values) equate to true.

One can see this as (in is not zero OR out is not zero), as neither value is zero, both tests equate to true, becoming (true OR true) - obviously this in turn is true.

The second half of the if statement is little different

 ( '$' || 'Y' || 'E' )

This will also equate to being ( true OR true Or True ), thus always equates to true eg:

( '$' != 0 || 'Y' != 0 || 'E' != 0)

Finally this breaks down to your code reading as such:

 if ((in_currency != 0 || out_currency =! 0) != ( '$' != 0 || 'Y' != 0 || 'E' != 0)) 

 if ((true || true) != ( true || true || true)) 
 if ((true) != (true))

Obviously, this will always equate to false, as true is never not equal to true (unless some universal constant changes ;-)

The result is your flag is always false. - always

The line I've given you to replace it, instead tests every possible case scenario separately and compared the result.

you can read this as

  if 
      in_currency **is equal to** '$' *or*  
      in_currency **is equal to** 'Y' *or*
      in_currency **is equal to** 'E'
    or 
      out_currency **is equal to** '$' *or*  
      out_currency **is equal to** 'Y' *or*
      out_currency **is equal to** 'E'
 then flag = 0;

[EDIT]

the user Turboc answered below that you may wish to use && (and) rather than or between these two statements.(please forgive me placing this information here, but I cannot yet comment on an answer)

This depends on how you intend for your statement to evaluate - if you wanted the flag to equal false (0) if the currency was any of the above, for output or input, then this is not the case).

However, if you wish the statement to be true as long as the currency is one of the above you would use the following

  if 
      in_currency **is equal to** '$' *or*  
      in_currency **is equal to** 'Y' *or*
      in_currency **is equal to** 'E'
    and 
      out_currency **is equal to** '$' *or*  
      out_currency **is equal to** 'Y' *or*
      out_currency **is equal to** 'E'
 then flag = 1;

However, if this is the case, you may wish to decide what answer is a more valid solution to your problem.

I'd like to upvote (and if you edit, I still might), but your translation of `if ((in || out) != ('A' || 'B' || 'C'))` doesn't work for me. I agree 100% the original is broken (but I'll come back to your description of how it is broken — nits to pick there, too). The original is likely to be intended to mean: _If `in` is not one of 'A' or 'B' or 'C', or if `out` is not one of 'A' or 'B' or 'C', then set `flag` to 0._ Your translation is missing the negation. You need something more like `if ((in != 'A' && in != 'B' && in != 'C') || (out != 'A' && out != 'B' && out != 'C')) flag = 0;`, I think. — Jonathan Leffler, Jul 13 '14 at 02:32
Your analysis of `(in_currency || out_currency)` as _is the same as typing `(in_currency == true || out_currency == true)`_ worries me too. A more precise characterization would be `(in_currency != 0 || out_currency != 0)`. The difference arises because `true` is defined via `` as `1`. Your translation only generates a true if `in_currency` is 1 or `out_currency` is 1; however, the original code generates true if `in_currency` is any non-zero value or if `out_currency` is any non-zero value — it only generates false if both variables are 0. Please be very careful! — Jonathan Leffler, Jul 13 '14 at 02:38
@JonathanLeffler , I appreciate the constructive critique. To your first point, I agree my phrasing was a little - complex. As to your second, that was actually my own misunderstanding - but that's what I get for not reading the standards. I'll be sure to correct it shortly. — John Bargman, Jul 18 '14 at 12:05

score 1 · Accepted Answer · edited Jun 20 '20 at 09:12

Converting some comments and other notes into an answer

These statements: char $ = $; char Y = Y; char E = E; invoke undefined behaviour (or, at the least, achieve nothing useful) by initializing the variable with their own uninitialized values. The variable $ isn't valid in standard C; you can probably find a C compiler that allows it (especially if you work on VMS). That, sadly, isn't a good start. The fact that the rest of the code doesn't use the variables is an additional problem. You can simply delete them.

Note that if the user enters 'source currency' as GPB, the new_line variable isn't going to contain a newline. You'd probably do better with using " %c" (space, percent, 'c') as the format string; it'll skip (optional) leading white space (blanks, tabs, newlines), and then one character. Repeat for the destination currency; it will skip the newline you expect and then read the next non-white space character. There's very little point in the "%f%c" format; it doesn't guarantee that you'll read a newline (and you don't check that it does).

Additionally, as others also pointed out, the condition:

if ( ( in_currency || out_currency ) != ( '$' || 'Y' || 'E' ) )

does not do what you want it to do. Because of the way C works, the RHS of the != evaluates to true (because '$' is not zero), and the LHS of it evaluates to true unless both in_currency and out_currency are zero, which is very unlikely (many people don't know how to type an ASCII NUL — aka the zero byte or '\0' — at a keyboard).

As I noted in a comment:

The original is likely to be intended to mean: _If in_currency is not one of '$' or 'Y' or 'E', or if out_currency is not one of '$' or 'Y' or 'E', then set flag to 0.

You need something more like

if (( in_currency != '$' &&  in_currency != 'Y' &&  in_currency != 'E') ||
    (out_currency != '$' && out_currency != 'Y' && out_currency != 'E'))
    flag = 0;

I got to see a version of the condition written as:

if (( in_currency != '$' ||  in_currency != 'Y' ||  in_currency != 'E' ) ||
    (out_currency != '$' || out_currency != 'Y' || out_currency != 'E' ))
    flag = 0;

This does not work properly. Think about it: suppose in_currency is Y (yen). The term in_currency != '$' will be true (because the value is Y), so the overall condition will be true (without any further terms being evaluated) and flag will be set to zero. If you had && separating the in_currency terms, then in_currency != '$' would be true, but in_currency != 'Y' would be false, so the (in_currency != '$' && in_currency != 'Y' && in_currency != 'E') part of the condition would be false — which is good because the currency code is valid. It would then go on to evaluate the out_currency terms (also with &&) and provided that the out_currency value was correct, would also pass.

Note that a condition of the form (a != v1 || a != v2) where the same variable a is compared to two distinct values v1 and v2 is always tautologously true. Beware, common English usage is a little slippery in this area; you have to be very careful and fastidious getting conditions correct. You also need to know De Morgan's Theorems (q.v.).

There are other issues one can pick with your code. For example, your error message is not very informative. You require the user to continue entering data after you've got enough information to know that it is invalid. You'll need to make sure your code handles the case where the user elects to convert from Euros to Euros (or, more generally, enters the same currency code twice).

Writing good code is, in part, an exercise in understanding how the user could subvert your best intentions, and heading off trouble before it can happen.

Using boolean flag to detect input error, C

2 Answers2