How on earth?!

OK, I have this simplest possible function and am getting very strange results:

void OpenSwitch(int OnOff) {
Serial.print(OnOff);
if(OnOff=1) {
digitalWrite(Switch,HIGH);
}
if(OnOff=2) {
Serial.print("in else");
digitalWrite(Switch,LOW);
}
}

If I call OpenSwitch(1), I'm getting the result "1in else"

What the *&$#!?

if(OnOff=1) 

if(OnOff=2)

You probably meant:

if(OnOff == 1)

if(OnOff == 2)

Lefty

backgrounder

The point is that in C if (onoff = 1) is a legal C statement which is expanded to

if ( (onoff = 1) != 0 )

onoff is assigned a value which is compared to 0 (zero) , if not zero it is considered true.


A good programming practice is to write

if (1 == OnOff) ... // first the const then the var

if one = is ommitted you get

if (1 = OnOff) .... which generates a syntax error

I might add that a boolean for an on/off state would be more intuitive and easier to handle with less change for misunderstanding.

I might add that a boolean for an on/off state would be more intuitive and easier to handle with less change for misunderstanding

But it still doesn't stop people writing if (onOff = true) :stuck_out_tongue_closed_eyes:

haha :slight_smile:

Yeah, thats a classic variable name giving "fail", straight out of the book.
LOL

Ah yes of course... you would think I might assign the simplest possible error to the simplest possible function :slight_smile:

Now that I've made this error 100 times maybe I can avoid the 101st? I think I'll create a flash card and tape it to my bench.

Thanks for keeping me sane folks!

braddo_99:
Ah yes of course... you would think I might assign the simplest possible error to the simplest possible function :slight_smile:

Now that I've made this error 100 times maybe I can avoid the 101st? I think I'll create a flash card and tape it to my bench.

Thanks for keeping me sane folks!

A method that some folks use to avoid this type of error/mistake is to
put the constant value on the left instead of the right.
i.e.

 if(1 == OnOff) {

that way if you accidentally use = instead of == the compiler will generate an error.

--- bill

@bperrybap
Psst, see reply #2

braddo_99:
void OpenSwitch(int OnOff) {
Serial.print(OnOff);
if(OnOff=1) {
digitalWrite(Switch,HIGH);
}
if(OnOff=2) {
Serial.print("in else");
digitalWrite(Switch,LOW);
}
}

When you say "in else" I see no "else" there.

How about getting rid of those pesky "=" signs altogether? When you call something OnOff but make it an int, that implies that the switch can be on, or off, ... or lots of other things. For simple on/off states use a boolean. Then you can test it without needing "==".

const byte Switch = 13;

void OpenSwitch(const boolean OnOff) 
  {
  Serial.print(OnOff, DEC);
  if (OnOff)
    {
    digitalWrite(Switch,HIGH);
    }
 else
    {
    Serial.print("in else");
    digitalWrite(Switch,LOW);
    }
}  // end of OpenSwitch

void setup ()
{
  Serial.begin (115200);
  OpenSwitch (true);
}  // end of setup

void loop () {}

Thanks Nick et al for the suggestions and continued help.

Actually that function was a throw-away, and it's no longer even in the project. I was just trying to test out my configuration of mosfets and relays before starting up with the more elaborate handling code. Then it didn't work for a while due to the stupid assignment error. Initially I had an if..else statement, but when that didn't work, I thought maybe I had a syntax problem with if.. else, and the debug statement stayed around after I substituted two if statements.

Several folks have recommended the if "(1==OnOff)" approach and I will try to put this into practice... but then I'll still have to remember when to write it forward versus backward. In fact I'm forgetting even while writing this :slight_smile:

Probably easy to forget because if it were me writing the language (god forbid), I might have set the convention the opposite way. To me the double "=" seems to imply emphasis, i.e. I really, r==ally want you to set this variable :slight_smile:

I have the same problems with the digitalWrite(MyPin,HIGH) convention, wreaks havoc troubleshooting what should be simple programs.

Have fun out there!

braddo_99:
Several folks have recommended the if "(1==OnOff)" approach and I will try to put this into practice... but then I'll still have to remember when to write it forward versus backward. In fact I'm forgetting even while writing this :slight_smile:

Probably easy to forget because if it were me writing the language (god forbid), I might have set the convention the opposite way. To me the double "=" seems to imply emphasis, i.e. I really, r==ally want you to set this variable :slight_smile:

There are situations where the single = is (was) actually desired.
Years ago, 20+ years back, if you wrote your code like this:

var = var2;
if(var)

you did not get as good of code if wrote it like this:

if(var = var2)

With todays optimizers, it no longer matters so it may be better
to avoid the use of the assignments inside ifs.

When using variables, reversing the order will not detect the accidental
issue.

i.e if you meant

if(var == var2)

and accidentally typed: (with the reversed order but mistyped ==)

if(var2 = var)

You will get no error and corrupt var2.

The real answer is to have the compiler detect assignments inside if statements for you.
The compiler can generate a warning for using assignments inside if statements,
unfortunately, the arduino IDE disables all warnings and there is no way to turn
them back on without patching the JAVA code in the IDE.

If you find that this it is a really big hurdle that you continually
struggle with, you could always define your own "=="....
While it would look a bit different and not be standard, you could do something like:

#define EQUALS ==

or

#define EQUALTO ==

Then you would use those instead of == for comparison.
i.e.

if(var EQUALS 6)

or

if(var EQUALTO var2)

Then there is no issues with ordering.

--- bill

Hey Bill, that's a cool idea - I like it. However, I'm sure it's just an issue of not having written enough code recently, and I'll pick it up with repetition.