Pages: [1] 2   Go Down
Author Topic: Redundant variables  (Read 891 times)
0 Members and 1 Guest are viewing this topic.
London
Offline Offline
Edison Member
*
Karma: 23
Posts: 1056
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

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

Sydney, Australia
Offline Offline
Edison Member
*
Karma: 27
Posts: 1179
Big things come in large packages
View Profile
WWW
 Bigger Bigger  Smaller Smaller  Reset Reset

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.
« Last Edit: August 13, 2013, 05:57:31 am by marco_c » Logged

Arduino libraries http://arduinocode.codeplex.com
Parola hardware & library http://parola.codeplex.com

Houston, Texas, USA
Offline Offline
Full Member
***
Karma: 2
Posts: 134
View Profile
WWW
 Bigger Bigger  Smaller Smaller  Reset Reset

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".
Logged

engineering is the art of planning and forethought
http://www.keywild.com

Global Moderator
Dallas
Offline Offline
Shannon Member
*****
Karma: 176
Posts: 12283
View Profile
WWW
 Bigger Bigger  Smaller Smaller  Reset Reset

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:
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:
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.
Logged

Cincinnati, OH
Offline Offline
Sr. Member
****
Karma: 15
Posts: 456
I'm not bossy...I just know what you should be doing.
View Profile
WWW
 Bigger Bigger  Smaller Smaller  Reset Reset

@lewtwo: You can achieve the same affect with

Code:
#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).
Logged

Global Moderator
UK
Offline Offline
Brattain Member
*****
Karma: 238
Posts: 24317
I don't think you connected the grounds, Dave.
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

@lewtwo: You can achieve the same effect with
Code:
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:
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 ()
{ }

« Last Edit: August 13, 2013, 06:43:36 am by AWOL » Logged

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

UK
Offline Offline
Faraday Member
**
Karma: 92
Posts: 3969
Where is your SSCCE?!?!
View Profile
WWW
 Bigger Bigger  Smaller Smaller  Reset Reset

I will generally always use:
Code:
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 smiley-wink
Logged

Why not visit my eBay shop? http://stores.ebay.co.uk/Majenko-Technologies
Replacement for the Arduino IDE: UECIDE - Proper serial terminal, graphing facilities, plugins, overhauled internals.
Java isn't bad in itself, but it has enabled morons to write programs.

Global Moderator
Offline Offline
Brattain Member
*****
Karma: 452
Posts: 18694
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

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:
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:
Binary sketch size: 2,164 bytes (of a 32,256 byte maximum)

Generated code (cycle counts in brackets):

Code:
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:
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:
Binary sketch size: 2,164 bytes (of a 32,256 byte maximum)

Generated code (cycle counts in brackets):

Code:
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:
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:
Binary sketch size: 2,164 bytes (of a 32,256 byte maximum)

Generated code (cycle counts in brackets):

Code:
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.
Logged

Idaho, US
Offline Offline
God Member
*****
Karma: 19
Posts: 859
Special User
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

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!
Logged

Offline Offline
Edison Member
*
Karma: 18
Posts: 1170
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

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

UK
Offline Offline
Faraday Member
**
Karma: 92
Posts: 3969
Where is your SSCCE?!?!
View Profile
WWW
 Bigger Bigger  Smaller Smaller  Reset Reset

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"...
Logged

Why not visit my eBay shop? http://stores.ebay.co.uk/Majenko-Technologies
Replacement for the Arduino IDE: UECIDE - Proper serial terminal, graphing facilities, plugins, overhauled internals.
Java isn't bad in itself, but it has enabled morons to write programs.

Idaho, US
Offline Offline
God Member
*****
Karma: 19
Posts: 859
Special User
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

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.
Logged

Global Moderator
Offline Offline
Brattain Member
*****
Karma: 452
Posts: 18694
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

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

That's funny too. Funnier even. smiley
Logged

Global Moderator
Offline Offline
Brattain Member
*****
Karma: 452
Posts: 18694
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

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.
Logged

Tulsa, OK
Offline Offline
Full Member
***
Karma: 1
Posts: 146
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

Don't forget about booleans!
Logged

Pages: [1] 2   Go Up
Jump to: