Give me a Clue About if Statements

• Give me a Clue About if Statements
  Posted by Roy Batty on 11 Jan, 2017 01:21
• So long, and thanks for all the fish.
• #1 Reply
  Posted by hendorog on 11 Jan, 2017 01:33
• One way is to use an 'else' in front of the second 'if'.
  If OnOff will only ever be 0 or 1 then you can just use 'else' on its own and remove 'if (OnOff ==1)'
  
  

  Code: [Select]

  if (something or other) {
   if (OnOff == 0) {
        OnOff = 1;
        ______;
        ______;
   } else if (OnOff == 1) {
       OnOff = 0;
       ______;
       ______;
   }
}

• #2 Reply
  Posted by ChristofferB on 11 Jan, 2017 01:34
• Another way is to use the 'switch' statement. If I recall correctly, it ends if the condition of a level of the switch is true.
• #3 Reply
  Posted by Mr.B on 11 Jan, 2017 01:41
• There are multiple ways to skin that cat.
  My way would be as follows and I would also declare OnOff as a boolean rather than an int
  

  Code: [Select]

  if (something or other) {
   if (OnOff) {
        OnOff = false;
        ______;
        ______;
   } else {
       OnOff = true;
       ______;
       ______;
   }
}

• #4 Reply
  Posted by iaeen on 11 Jan, 2017 02:10
• Quote from: ChristofferB on 11 Jan, 2017 01:34

  Another way is to use the 'switch' statement. If I recall correctly, it ends if the condition of a level of the switch is true.
  

  
  My C us unfortunately rusty, but I believe you have to manually break out of a switch. That would work for the OP since none of the other conditions matter after the break:
  

  Code: [Select]

  Switch{
    Case OnOff;
        OnOff=0;
        //other stuff goes here
    Break;
    Case !OnOff;
        OnOff=1;
        //again with the other stuff
    Break;
}

  Without the breaks, you would execute all the code in the switch starting with the first case that evaluates true. For example:
  

  Code: [Select]

  Switch {
    Case true;
        foo="this won't be the final value of the variable";
    Case false;
        foo="This is will be the final value";
}

  I'd still probably use an if/else statement. Most people only use switches if there are a bunch of cases.
• #5 Reply
  Posted by Mr.B on 11 Jan, 2017 03:25
• In your original code, the second statement would always get executed if the first statement was executed.
  This is because the first statement would set the OnOff variable to a value that evaluates to True in the second test.
• #6 Reply
  Posted by donotdespisethesnake on 11 Jan, 2017 11:13
• OnOff is a terrible name of course. LightOn would be much better, and make it a bool.
• #7 Reply
  Posted by MattHollands on 11 Jan, 2017 13:31
• Quote from: Roy Batty on 11 Jan, 2017 03:17

  Thanks, all. I just want to know for sure: will it read and execute the second part of the statement even after reading and executing the second part? I'm not concerned about finding a way around it, because using "else" seems to work.
  
  I just assumed that if you put two "if" statements in series, intending to have a disjunction, the computer would only read one, but I guess that's not true. I guess the first if has no effect on the second, and "else" really means, "If that didn't work, try this."
  

  
  The computer is dumb and everything is sequential. First the computer looks at the first if-statement and evaluates it. When it's done with the first if-statement, it completely forgets about it. Then it looks at the second if statement - it doesn't think "ah I've already evaluated this variable in this loop, I'll ignore this" - it does exactly as it is told and evaluates the second if statement.
  
  If you put 1 million exactly the same if-statements in a row, it would evaluate them all.
  
  (Yes, compiler optimisation may break some of these rules - but there are very rare occasions when you will need to worry about this).
  
  Basically for a variable with two values I would use:
  
  

  Code: [Select]

  if (something or other)
{
   if (OnOff == 0) {
        OnOff = 1;
        ______;
        ______;
    }
    else
    {
       OnOff = 0;
       ______;
       ______;
   }
}


  (also as mentioned before I would prefer to use a bool than an int)
• #8 Reply
  Posted by AndyC_772 on 11 Jan, 2017 17:27
• You might enjoy VHDL; logic is evaluated very differently.
  
  For example:
  
  

  Code: [Select]

  IF rising_edge (clk) THEN
  IF LightOn = '1' THEN
    LightOn <= '0';
    ...do some stuff...
  END IF;

  IF LightOn = '0' THEN
    LightOn <= '1';
    ...do other stuff...
  END IF;
END IF;

  ... actually does work how you'd hope it might, rather than how a programmer more familiar with languages like C (and the various mistakes that beginners often make) would expect it to.
  
• #9 Reply
  Posted by rstofer on 11 Jan, 2017 17:28
• There is a school of thought that conditionals like OnOff == 0 are evil.  Here's why...
  
  When you write
  

  Code: [Select]

  if (OnOff == 0)
it sometimes happens that you actually type
  

  Code: [Select]

  if (OnOff = 0)
which unconditionally sets OnOff to 0 and the expression evaluates to false.  This kind of thing is surprisingly hard to debug and happens far more often than you would think.
  
  So, the idea is to turn the expression around
  

  Code: [Select]

  if (0 == OnOff)   // this will compile and work properly
whereas
  

  Code: [Select]

  if (0=OnOff) // this won't even compile

  I am trying to retrain myself to use the alternate expression.  I wish I could say it is working...
  
• #10 Reply
  Posted by Ian.M on 11 Jan, 2017 17:37
• If its a boolean value then simply use either
  

  Code: [Select]

  if(OnOff) // test if TRUEor
  

  Code: [Select]

  if(!OnOff) // test if FALSEThe == doesn't add any clarity, is more verbose and increases the risk of a typo resulting in an = assignment.
     
• #11 Reply
  Posted by djacobow on 11 Jan, 2017 17:57
• Quote from: Ian.M on 11 Jan, 2017 17:37

  If its a boolean value then simply use either
  

  Code: [Select]

  if(OnOff) // test if TRUEor
  

  Code: [Select]

  if(!OnOff) // test if FALSEThe == doesn't add any clarity, is more verbose and increases the risk of a typo resulting in an = assignment.
   
  

  
  This is idiomatic c and you should code this way as long as the type can be interpreted as a boolean, regardless of what type it actually is. Code like:
  
  
  if (a == 0) { }
  
  
  give me the shakes, and
  
  
  int *p;
  ...
  if (p == NULL) { }
  
  
  causes seizures.
  
  Don't do it! :-)
  
• #12 Reply
  Posted by hamster_nz on 11 Jan, 2017 20:20
• Quote from: djacobow on 11 Jan, 2017 17:57

  Quote from: Ian.M on 11 Jan, 2017 17:37

  If its a boolean value then simply use either
  

  Code: [Select]

  if(OnOff) // test if TRUEor
  

  Code: [Select]

  if(!OnOff) // test if FALSEThe == doesn't add any clarity, is more verbose and increases the risk of a typo resulting in an = assignment.
   
  

  
  This is idiomatic c and you should code this way as long as the type can be interpreted as a boolean, regardless of what type it actually is. Code like:
  
  
  if (a == 0) { }
  
  
  give me the shakes, and
  
  
  int *p;
  ...
  if (p == NULL) { }
  
  
  causes seizures.
  
  Don't do it! :-)
  

  
  A quick puzzle - what is wrong with this:
  
  

  Code: [Select]

    int f;
  if(!(f = open("file",O_RDONLY))) {
     fatal_error("Unable to open 'file'");
  }

  
  with that in mind, which do you hate more, and why...
  
  Option A
  

  Code: [Select]

    FILE *f;
  f = fopen("file","rb");
  if(f == NULL) {
     fatal_error("Unable to open 'file'");
  }

  vs
  
  Option B
  

  Code: [Select]

    FILE *f;
  f = fopen("file","rb");
  if(!f) {
     fatal_error("Unable to open 'file'");
  }

  vs
  
  Option C
  

  Code: [Select]

    FILE *f;
  if(!(f = fopen("file","rb"))) {
     fatal_error("Unable to open 'file'");
  }

  vs (some insane Perl inspired crap...)
  
  Option D
  

  Code: [Select]

    FILE *f;
  (f = fopen("file","rb")) || fatal_error("Unable to open 'file'");

  I prefer the first, because I like the "try something. Did it return an error? If so handle the exception" pattern.
  
  EDIT: Actually tested code and fixed dumb syntax errors
  
• #13 Reply
  Posted by MarkL on 11 Jan, 2017 20:52
• If you're using GCC, I would suggest you always compile with "-Wall -Werror" (or some reasonable subset of warning options if "all" is too picky for you).
  
  Then, code like this with unintended assignments:
  

  Code: [Select]

  #include <stdio.h>

int main()
{
    int a;
    int *p;

    if (a = 0) { }
    if (p = NULL) { }

    return 0;
}
will result in warning messages and stop the compilation, like so:
  
  

  Code: [Select]

  zz.c: In function ‘main’:
zz.c:8: warning: suggest parentheses around assignment used as truth value
zz.c:9: warning: suggest parentheses around assignment used as truth value

• #14 Reply
  Posted by MarkL on 11 Jan, 2017 23:44
• My apologies for the off-topic post.  henderog had the answer in the first reply to your post.  Use an "else".
• #15 Reply
  Posted by Kremmen on 12 Jan, 2017 00:30
• The discussion went off on a tangent as always happens. But from the already given answers to the original question, do take home a couple of good bits:
  - learn to use the boolean type (name of boolean type is 'bool' in Arduino) for variables that are only meaningful as 'on'/'off' or true/false. A boolean can have just those 2 values and that will spare you all kinds of stupid errors later. What if your OnOff accidentally gets a value 3 and you always compare to 0 or 1? Can't happen with a boolean type.
  - Name Booleans to indicate the true case. So don't do GizmoOnOff, but do gizmoOn.
  - Then compare against the true value: if ( gizmoOn ) ... Self evident don't you think. In rare cases compare against the inverted value: if ( !gizmoOn ) ... (i.e. if gizmo off...)
  
  Now that i got going, other similar pieces of sagely advice:
  - do not embed magic numbers in your code. Two weeks after writing a piece of software you will not recall what all the numbers meant again, that you have sprinkled around the code. Instead do #define your magick numbers as named constants thus:
  #define MAX_GIZMOS 10
  #define PULSE_DELAY 10
  etc.
  In the code you use the #defined names: if ( numberOfActualGizmos >= MAXGIZMOS ) alarm();
  Now when you later want to increase the maximum number of gizmos to 11, you just change the #define instead of hunt all over the code for just those 10's that are related to amount of gizmos.
  
  - another very useful practice is to learn to use enumerated types. Sounds cryptic but isn't and helps the compiler to report certain kinds of error not easily spotted otherwise. Not to mention that it makes life easier for you the programmer.
  Enumerated type are very useful when you hvae a small number of exclusive distinct values and other values don't make sense. A classic example is how to identify e.g. colors. You could assign 1 to red, 2 to blue and so on. But later in your code how will you recall what number was which color? You could #define the values sure, but easier to just declare 'enum color {red, blue, green yellow cyan, magenta};' That way the compiler remembers the values for you, and you just call the colors by name. Now you can declare variable to be of type 'color': color background; color foreground; etc. Later just say e.g. 'background = blue;' to assign bg color.
  
  
• #16 Reply
  Posted by djacobow on 12 Jan, 2017 00:32
• Quote from: hamster_nz on 11 Jan, 2017 20:20

  
  with that in mind, which do you hate more, and why...
  
  

  
  I liked B the best, but prefer
  
  
  FILE *f = fopen("some.file","rb");
  if (!f) {
    ...
  }
  
  
  I guess I can't justify it with any hard rule. I think combining declaration and assignment is a good use of vertical space, but putting assignment in a conditional is not.
  
  NULL, though, bothers me. It's usually just a macro for (void *)0. The C standard makes it clear that the value 0 in a pointer context (even without the cast to void*) is a null pointer. So, though, from a hardware perspective, one could have a pointer that is not null that points to memory location 0, the C language is not part of that universe -- thus stealing a precious byte of usable address space from us but sparing us from having to look at NULL and all its many homebrew nonsense variants.
  
  I'm not even sure what work the (void *) cast is doing in the macro definition of NULL. At first, I thought it  was so that you would get a warning or error if you tried to assign NULL to something that is not a pointer. But, nope, that works just fine, too.
  
  
• #17 Reply
  Posted by djacobow on 12 Jan, 2017 00:43
• Quote from: Kremmen on 12 Jan, 2017 00:30

  The discussion went off on a tangent as always happens. But from the already given answers to the original question, do take home a couple of good bits:
  - learn to use the boolean type (name of boolean type is 'bool' in Arduino) for variables that are only meaningful as 'on'/'off' or true/false. A boolean can have just those 2 values and that will spare you all kinds of stupid errors later.
  

  
  Sorry, I thought tangents were fun?
  
  I am just chiming in again to say that the original question was about "C", which until C99 had no boolean type, though you could use any integer type and it works just like a bool as long as you only do booly stuff to it.
  
  I'm not certain what problem C99's stdbool.h is solving, but I guess the world got tired of seeing some variation of
  
  typedef enum { false, true } bool;
  
  in everyone's code. I guess they could also be wanting to harmonize with C++.
  
  
  
  
• #18 Reply
  Posted by Ian.M on 12 Jan, 2017 01:52
• Quote from: djacobow on 12 Jan, 2017 00:43

  I am just chiming in again to say that the original question was about "C", which until C99 had no boolean type, though you could use any integer type and it works just like a bool as long as you only do booly stuff to it.
  

  I started the bool branch of the tangent, but in my defence I said "boolean values" which can be stored in any K&R or C89 integer typed variable, and didn't mention the C99 'bool' boolean type.
• #19 Reply
  Posted by boffin on 12 Jan, 2017 02:04
• Quote from: Roy Batty on 11 Jan, 2017 01:21

  Maybe I should put this question on a programming forum, but I thought I'd give it a shot here. Sorry if it's out of place.
  
  I am working on Arduino programming, and I've been dealing with if statements. I have a very basic question.
  
  Let's say I have some value I'm reading. It's an int, and I'll call it "OnOff." Suppose it tells you whether a light is on or off. If it's on, you want to do something. If it's off, you want to do something else.
  
  You write an if statement. The statement has to reflect the fact that you've changed the system, so it has to change the value of "OnOff" before it's over.
  
  So you have something like:
  
  if (something or other) {
     if (OnOff == 0) {
          OnOff = 1;
          ______;
          ______;
                             }
  
     if (OnOff == 1_ {
         OnOff = 0;
         ______;
         ______;
                           }
     }
  
  The idea is that you do one thing if OnOff is 0, and you do something else if OnOff is 1, and then you change the value of OnOff in order to reflect that something has been done.
  
  The question is this: will the second half of the if statement above read the change in OnOff (made in the first half) before running? You want the top part (not the bottom part) to run if OnOff is 0, so you would not want the bottom part to read the bit where OnOff is changed to 1 and start running.
  

  Yes, which is why there are else statements. What you're looking for is
  
  

  Code: [Select]

  if (something or other) {
   if (OnOff == 0) {
        OnOff = 1;
        ______;
        ______;
   }
   else {               // this block runs if the above block didn't
       OnOff = 0;
       ______;
       ______;
   }
}


• #20 Reply
  Posted by Kremmen on 12 Jan, 2017 08:24
• Quote from: djacobow on 12 Jan, 2017 00:43

  [...]
  Sorry, I thought tangents were fun?
  
  I am just chiming in again to say that the original question was about "C", which until C99 had no boolean type, though you could use any integer type and it works just like a bool as long as you only do booly stuff to it.
  
  I'm not certain what problem C99's stdbool.h is solving, but I guess the world got tired of seeing some variation of
  
  typedef enum { false, true } bool;
  
  in everyone's code. I guess they could also be wanting to harmonize with C++.
  

  
  Oh, i didn't say tangents can't be fun. No problem there - it was jus an observation of how things usually go.
  
  I confess being one of those who has gotten deadly tired of seeing all possible permutations of the way in which the enumeration of {false, true} can be expressed and fail to be compatible with any of the others. But that was not the point. Instead the point was just to illuminate the whole concept of using variables as something other than numbers or strings of characters.
• #21 Reply
  Posted by T3sl4co1l on 12 Jan, 2017 14:57
• Note that you can put the assignment of OnOff outside the inner if statement.  That is,
  
  

  Code: [Select]

  if (something) {
    OnOff = !OnOff // use a bool type for semantic correctness
    if (OnOff) {
        // OnOff was off and now it's on. Do the "off" stuff.
    } else {
        // OnOff was on and now it's off. Do the "on" stuff.
    }
}

  But this is confusing, because the inner if statement is upside-down so to speak.  We can simply defer changing OnOff to later,
  
  

  Code: [Select]

  if (something) {
    if (OnOff) {
        // OnOff is on. Do the "on" stuff.
    } else {
        // OnOff is off. Do the "off" stuff.
    }
    OnOff = !OnOff
}

  This is fine, but isn't foolproof: a pathological case could have a break or goto inside the if statement, which gleefully leaves before the OnOff assignment is evaluated.  Simple enough: don't use goto, and use break only when appropriate (most often in switch statements, or when you need to exit a for or while statement prematurely).
  
  
  
  An aside: switch statements are quite inefficient until you need a large list of consecutive options (ten or more, say).  Nothing wrong with the semantics -- it's just slow and bulky until you need a lot of options.
  
  Tim
• #22 Reply
  Posted by rstofer on 12 Jan, 2017 15:23
• I like Option A
  
  

  Code: [Select]

  FILE *f;
  f = fopen("file","rb");
  if(f == NULL) {
     fatal_error("Unable to open 'file'");
  }

  I may not know what fopen returns but I'm pretty sure that NULL is an error.  I really don't know how to evaluate (!(fopen....) without stopping to think about it.  Let's see, an error returns 0 and the ! inverts it thus the conditional is TRUE so we take the branch which is an error trap...  Not my favorite although I have used it...
  
  The only thing I would suggest changing is the 'if' statement:
  
  

  Code: [Select]

  FILE *f;
  f = fopen("file","rb");
  if(NULL == f) {
     fatal_error("Unable to open 'file'");
  }

  I concede that it doesn't read as well as the first instance above.
  
• #23 Reply
  Posted by rstofer on 12 Jan, 2017 15:34
• OP, there are some interesting tidbits in this thread which were enumerated in an earlier post.  Don't get discouraged, most of us have been making programming mistakes for decades.  I started messing up in '70 so almost 5 decades of messy code.
  
  Over the years you come up with ways of doing things that you like.  There are almost always multiple ways of arriving at a result, some of which actually make sense.  All of the tidbits above will make sense at some point as they will lead you to cleaner code and code more likely to actually work.
  
  C, especially, has a lot of alternative methods for doing just about anything.  It takes time...  Maybe put a copy of K&R under your pillow at night.
  
  "The C Programming Language" Kernighan and Ritchie (K&R)
  
• #24 Reply
  Posted by jnz on 12 Jan, 2017 21:17
• Quote from: Roy Batty on 11 Jan, 2017 22:44

  This thread has been over my head since some time yesterday.
  

  
  They like to do that here.
  
  Read it all a few times and move on, come back to it if you're having problems. Simple questions get drilled down into fairly complex quickly. Try posting stuff like this in the Beginner's Section.