Go Down

Topic: Redundant variables (Read 1 time) previous topic - next topic

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.
Code: [Select]
buttonPress = digitalRead(pin7);
if (buttonPress==HIGH){...
Why not?
Code: [Select]
if (digitalRead(pin7)==HIGH){...Or even?
Code: [Select]
if (digitalRead(pin7)){...
Another example is:
Code: [Select]
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.

marco_c

#1
Aug 13, 2013, 04:29 am Last Edit: Aug 13, 2013, 12:57 pm by marco_c Reason: 1
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.
Arduino libraries http://arduinocode.codeplex.com<br />Parola for Arduino http://parola.codeplex.com

lewtwo

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".
engineering is the art of planning and forethought
http://www.keywild.com

Coding Badly

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.

Quote
Why not?


Debugging is a bit easier...
Code: [Select]
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.

Quote
Another example is:
Code: [Select]
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.

econjack

@lewtwo: You can achieve the same affect with

Code: [Select]
#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).

AWOL

#5
Aug 13, 2013, 01:33 pm Last Edit: Aug 13, 2013, 01:43 pm by AWOL Reason: 1
@lewtwo: You can achieve the same effect with
Code: [Select]
const byte LED = 13;

Quote
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
Code: [Select]
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 ()
{ }


"Pete, it's a fool looks for logic in the chambers of the human heart." Ulysses Everett McGill.
Do not send technical questions via personal messaging - they will be ignored.

majenko

I will generally always use:
Code: [Select]

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 ;)
Get 10% off all 4D Systems TFT screens this month: use discount code MAJENKO10

Nick Gammon


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

Code: [Select]

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:

Code: [Select]

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


Generated code (cycle counts in brackets):

Code: [Select]

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

Code: [Select]

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:

Code: [Select]

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


Generated code (cycle counts in brackets):

Code: [Select]

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

Code: [Select]

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:

Code: [Select]

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


Generated code (cycle counts in brackets):

Code: [Select]

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.




Quote

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:

Quote

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.
http://www.gammon.com.au/electronics

tylernt


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.
That was eye-opening. Thanks.

Those compiler writers are dang good at their job!

KeithRB

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?"

majenko



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



Surely you mean Ex, as in "has been"...
Get 10% off all 4D Systems TFT screens this month: use discount code MAJENKO10

tylernt

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.

Nick Gammon


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


That's funny too. Funnier even. :)
http://www.gammon.com.au/electronics

Nick Gammon


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.
http://www.gammon.com.au/electronics

cypherrage

Don't forget about booleans!

Go Up