Redundant variables

I've noticed that many people here, even some of the 'experts', use variables that can easily be eliminated, making for shorter, faster and more efficient code. E.g.

buttonPress = digitalRead(pin7);
if (buttonPress==HIGH){...

Why not?if (digitalRead(pin7)==HIGH){...Or even?if (digitalRead(pin7)){... Another example is:currentTime=millis();Why not just use millis()? After a while, currentTime is no longer the current time! Only if they are needed elsewhere in the code could there be any reason to use them. And millis() is always accessible, apart from during interupts.

For beginners assigning the variable may be clearer, as it breaks down the syntax into smaller steps. I agree that for advanced programmers this is not necessary and wasteful of resources.

I noticed this when I loaded the example program “blink.ino”.
I was wondering why it did not use a constant instead.
I thought that there might be some reason that the function required a parameter as a variable.
So I changed the declaration.
const int led = 13;

It works fine and is just as “user friendly”.

Henry_Best: I've noticed that many people here, even some of the 'experts', use variables that can easily be eliminated, making for shorter, faster and more efficient code.

Unless the wrong data-type is chosen, eliminating / using local variables makes no difference. The value has to be loaded in a register. All a local variable does is provide a name for that register.

Why not?

Debugging is a bit easier...

buttonPress = digitalRead(pin7);
Serial.println( buttonPress ); // Remove when finished debugging
if (buttonPress==HIGH){...

Well-named variables help to document the code.

Using variables helps provide a clean snapshot of the system state.

Another example is:currentTime=millis();Why not just use millis()?

Clean snapshot of the current time. Try riddling your PID algorithm with calls to millis; you will quickly find out why a clean snapshot is important.

@lewtwo: You can achieve the same affect with

#define LED 13

Both syntax versions "do away with" the variable led . While it makes little difference in the Arduino IDE, the #define, in effect, removes the variable from the symbol table. Therefore, symbolic debuggers no longer would have an lvalue to work with. Using const would at least let you observe the value of led (even though it isn't doing anything interesting).

@lewtwo: You can achieve the same effect with

const byte LED = 13;

I've noticed that many people here, even some of the 'experts', use variables that can easily be eliminated, making for shorter, faster and more efficient code

Hmm, let's see

const byte inputPin = 2;
const byte LEDpin = 13;
void setup ()
{
  pinMode (inputPin, INPUT);
  digitalWrite (inputPin, HIGH);
  pinMode (LEDpin, OUTPUT);
  digitalWrite (LEDpin, LOW);
#if 0
  int x = digitalRead (inputPin);
  if (x == HIGH)
#endif
#if 0
  if (digitalRead (inputPin) == HIGH)
#endif
 #if 1
  if (digitalRead (inputPin))
#endif
  {
    digitalWrite (LEDpin, HIGH); 
  }
}

void loop ()
{ }

I will generally always use:

unsigned long now = millis();

and then do all my comparisons and assignments using the variable "now". Not as a clean snapshot, although that is nice to have, but mainly because accessing a variable multiple times requires much less code than calling the millis() function multiple times. Yes, the C code may be slightly longer, but you can bet your ass the ASM code will be shorter.

Rule 1: If you're calling the same function twice from the same scope to get the same data then you're doing it wrong ;)

Henry_Best:
I’ve noticed that many people here, even some of the ‘experts’, use variables that can easily be eliminated, making for shorter, faster and more efficient code. E.g. …

Well we all know an expert is:

  • X: unknown
  • spurt: a drip under pressure

But putting that aside, let’s test your theory.


The “expert” way

const byte pin7 = 7; // !
void setup ()
  {
  Serial.begin (115200);
  }  // end of setup
void loop ()
  {
  byte buttonPress = digitalRead (pin7);
  if (buttonPress == HIGH)
    {
    Serial.println ("HIGH");
    }
  else
    {
    Serial.println ("LOW");
    }
  }  // end of loop

Code size:

Binary sketch size: 2,164 bytes (of a 32,256 byte maximum)

Generated code (cycle counts in brackets):

00000118 <loop>:
 118:   87 e0           ldi     r24, 0x07       ; 7   (1)
 11a:   0e 94 a6 00     call    0x14c   ; 0x14c <digitalRead>   (4)
 11e:   81 30           cpi     r24, 0x01       ; 1   (1)
 120:   29 f4           brne    .+10            ; 0x12c <loop+0x14>   (1/2)
 122:   8e e9           ldi     r24, 0x9E       ; 158   (1)
 124:   91 e0           ldi     r25, 0x01       ; 1   (1)
 126:   60 e0           ldi     r22, 0x00       ; 0   (1)
 128:   71 e0           ldi     r23, 0x01       ; 1   (1)
 12a:   04 c0           rjmp    .+8             ; 0x134 <loop+0x1c>   (2)
 12c:   8e e9           ldi     r24, 0x9E       ; 158   (1)
 12e:   91 e0           ldi     r25, 0x01       ; 1   (1)
 130:   65 e0           ldi     r22, 0x05       ; 5   (1)
 132:   71 e0           ldi     r23, 0x01       ; 1   (1)
 134:   0e 94 6d 03     call    0x6da   ; 0x6da <_ZN5Print7printlnEPKc>   (4)
 138:   08 95           ret   (4)

The “efficient” way

const byte pin7 = 7; // !
void setup ()
  {
  Serial.begin (115200);
  }  // end of setup
void loop ()
  {
  if (digitalRead (pin7) == HIGH)
    {
    Serial.println ("HIGH");
    }
  else
    {
    Serial.println ("LOW");
    }
  }  // end of loop

Code size:

Binary sketch size: 2,164 bytes (of a 32,256 byte maximum)

Generated code (cycle counts in brackets):

00000118 <loop>:
 118:   87 e0           ldi     r24, 0x07       ; 7   (1)
 11a:   0e 94 a6 00     call    0x14c   ; 0x14c <digitalRead>   (4)
 11e:   01 97           sbiw    r24, 0x01       ; 1   (2)
 120:   29 f4           brne    .+10            ; 0x12c <loop+0x14>   (1/2)
 122:   8e e9           ldi     r24, 0x9E       ; 158   (1)
 124:   91 e0           ldi     r25, 0x01       ; 1   (1)
 126:   60 e0           ldi     r22, 0x00       ; 0   (1)
 128:   71 e0           ldi     r23, 0x01       ; 1   (1)
 12a:   04 c0           rjmp    .+8             ; 0x134 <loop+0x1c>   (2)
 12c:   8e e9           ldi     r24, 0x9E       ; 158   (1)
 12e:   91 e0           ldi     r25, 0x01       ; 1   (1)
 130:   65 e0           ldi     r22, 0x05       ; 5   (1)
 132:   71 e0           ldi     r23, 0x01       ; 1   (1)
 134:   0e 94 6d 03     call    0x6da   ; 0x6da <_ZN5Print7printlnEPKc>   (4)
 138:   08 95           ret   (4)

Same sketch size, but the “more efficient” way seems to me to use one more clock cycle (instruction at 0x11e).


The “even more efficient” way

const byte pin7 = 7; // !
void setup ()
  {
  Serial.begin (115200);
  }  // end of setup
void loop ()
  {
  if (digitalRead (pin7))
    {
    Serial.println ("HIGH");
    }
  else
    {
    Serial.println ("LOW");
    }
  }  // end of loop

Code size:

Binary sketch size: 2,164 bytes (of a 32,256 byte maximum)

Generated code (cycle counts in brackets):

00000118 <loop>:
 118:   87 e0           ldi     r24, 0x07       ; 7   (1)
 11a:   0e 94 a6 00     call    0x14c   ; 0x14c <digitalRead>   (4)
 11e:   89 2b           or      r24, r25   (1)
 120:   29 f0           breq    .+10            ; 0x12c <loop+0x14>   (1/2)
 122:   8e e9           ldi     r24, 0x9E       ; 158   (1)
 124:   91 e0           ldi     r25, 0x01       ; 1   (1)
 126:   60 e0           ldi     r22, 0x00       ; 0   (1)
 128:   71 e0           ldi     r23, 0x01       ; 1   (1)
 12a:   04 c0           rjmp    .+8             ; 0x134 <loop+0x1c>   (2)
 12c:   8e e9           ldi     r24, 0x9E       ; 158   (1)
 12e:   91 e0           ldi     r25, 0x01       ; 1   (1)
 130:   65 e0           ldi     r22, 0x05       ; 5   (1)
 132:   71 e0           ldi     r23, 0x01       ; 1   (1)
 134:   0e 94 6d 03     call    0x6da   ; 0x6da <_ZN5Print7printlnEPKc>   (4)
 138:   08 95           ret   (4)

Same sketch size, but now we are down to the same clock cycles as the “expert” way.


The compiler aggressively optimizes. You are best off writing readable code. The experts write readable code because they know one day they’ll have to debug it. You can see that by trying to second-guess the compiler (second sketch above) you end up with code that is no smaller, but slower.


Why not just use millis()? After a while, currentTime is no longer the current time!

Yes, but if you are doing calculations on the time “at a certain point” you are probably best off using that time and not having two lots of the “current time”. It’s like the old saying:

Man with one watch is certain of the time. Man with two watches is not sure.

Plus, possibly more importantly, calling millis takes time. Since you are keen to write efficient code, you might consider that replacing multiple function calls with a single function call is more efficient. The function millis(), for example, turns interrupts off for a couple of instructions. Doing this multiple times means interrupts are off longer than they might need to be.

[quote author=Nick Gammon link=topic=182430.msg1351757#msg1351757 date=1376399154] The compiler aggressively optimizes. You are best off writing readable code. The experts write readable code because they know one day they'll have to debug it. You can see that by trying to second-guess the compiler (second sketch above) you end up with code that is no smaller, but slower.[/quote]That was eye-opening. Thanks.

Those compiler writers are dang good at their job!

Kernigham and Plauger again: "Everyone knows that debugging is twice as hard as writing a program in the first place. So if you are as clever as you can be when you write it, how will you ever debug it?"

[quote author=Nick Gammon link=topic=182430.msg1351757#msg1351757 date=1376399154]

Henry_Best: I've noticed that many people here, even some of the 'experts', use variables that can easily be eliminated, making for shorter, faster and more efficient code. E.g. ...

Well we all know an expert is:

  • X: unknown
  • spurt: a drip under pressure

[/quote] Surely you mean Ex, as in "has been"...

Speaking of "wasting" variables, I notice that a lot of people (even Arduino Examples) tend to use 'int' when a 'byte' (8-bit unsigned) or 'char' (8-bit signed) will do. As the Arduino has an 8-bit CPU, bytes/chars will execute faster than ints, as well as taking up less RAM.

And when 'int' isn't enough, they go straight to 'long' without checking to see if 'unsigned int' will work.

So while "optimizing" C code may not help much, optimizing variable use can help.

majenko: Surely you mean Ex, as in "has been"...

That's funny too. Funnier even. :)

tylernt: So while "optimizing" C code may not help much, optimizing variable use can help.

Agreed. And adding in "const" whenever you know something won't change.

Don't forget about booleans!

cypherrage: Don't forget about booleans!

What about them?

AWOL:

cypherrage: Don't forget about booleans!

What about them?

They waste space ;) An entire byte for a single bit's worth of data. Compress them all into bitfields using an anonymous struct inside a union :)

Compress them all into bitfields using an anonymous struct inside a union

And then I can use pointers to them? References?

Hmm something like this? http://www.cs.cf.ac.uk/Dave/C/node13.html

cypherrage: Hmm something like this? http://www.cs.cf.ac.uk/Dave/C/node13.html

Yup.

AWOL:

Compress them all into bitfields using an anonymous struct inside a union

And then I can use pointers to them? References?

To the byte / word that contains them, yes. Not to individual bits of course, they're bits.

But if you are running low on RAM and 7 bytes might make the difference between it working and it not working, compressing 8 booleans into a single byte is certainly worth considering.

It's also great for grouping related booleans together into a single unit. You can copy them en-masse by copying the whole byte. Store the whole lot in one cell of the EEPROM with a single write. All sorts of good things.