Are static variables in a function bad?

Question

For my code I use a couple of static variables. But when I search the internet, they seem "evil".

For example: I have a function which will be executed every loop (5ms looptime). to increase the speed I added some static variables to it. Is the way I am doing this bad? Are there simple solutions for it?

Code:

int ProgFormula() {
  static int StartPosition;
  static int DeltaPosition;
  static int LoopTime = 5;
  static int AccelerationLoops;
  static int TotalLoops;
  static int EndOne;

  static float VelocityLoop;
  static float VelocityFactor;

  int Setpoint;

  if (Iteration == -1) {
    StartPosition = PID.ActualPosition;
    DeltaPosition = abs(SetPosition - StartPosition);
    VelocityLoop = Velocity / (1000 / LoopTime);
    TotalLoops = DeltaPosition / VelocityLoop;
    if (TotalLoops <= (AccelerationLoops * 2)) {
      AccelerationLoops = (TotalLoops / 2);
      VelocityFactor = (VelocityLoop * 0.5 / AccelerationLoops) / 2;
    }
    else {
      AccelerationLoops = 70;

      VelocityFactor = VelocityLoop * 0.5 / AccelerationLoops;
    }
    TotalLoops = DeltaPosition / VelocityLoop + AccelerationLoops;
    EndOne = (VelocityFactor * pow(AccelerationLoops, 2));
  }
  else if (Iteration >= 0 && Iteration <= AccelerationLoops) {
    if (StartPosition < SetPosition) {
      Setpoint = VelocityFactor * pow(Iteration, 2) + StartPosition;
    }
    else {
      Setpoint = -VelocityFactor * pow(Iteration, 2) + StartPosition;
    }
  }
  else if (Iteration > AccelerationLoops && Iteration <= (TotalLoops - AccelerationLoops)) {
    if (StartPosition < SetPosition) {
      Setpoint = VelocityLoop * Iteration - EndOne + StartPosition;
    }
    else {
      Setpoint = -VelocityLoop * Iteration + EndOne + StartPosition;
    }
  }
  else if (Iteration >= (TotalLoops - AccelerationLoops) && Iteration <= TotalLoops) {
    if (StartPosition < SetPosition) {
      Setpoint = -VelocityFactor * pow((TotalLoops - Iteration), 2) + DeltaPosition + StartPosition;
    }
    else {
      Setpoint = VelocityFactor * pow((TotalLoops - Iteration), 2) - DeltaPosition + StartPosition;
    }
  }
  else {
    Setpoint = Setpoint;
  }
  return(Setpoint);
}

Is it better to avoid statics? If so why?

Thanks in advance, Koen

Answer 1

I made up a test by adding enough to the posted code to get it to compile:

float ActualPosition, SetPosition, Velocity;

int Iteration;

int ProgFormula() {

  static int StartPosition;
  static int DeltaPosition;
  static int LoopTime = 5;
  static int AccelerationLoops;
  static int TotalLoops;
  static int EndOne;

  static float VelocityLoop;
  static float VelocityFactor;

  int Setpoint;

  if (Iteration == -1) {
    StartPosition = ActualPosition;
    DeltaPosition = abs(SetPosition - StartPosition);
    VelocityLoop = Velocity / (1000 / LoopTime);
    TotalLoops = DeltaPosition / VelocityLoop;
    if (TotalLoops <= (AccelerationLoops * 2)) {
      AccelerationLoops = (TotalLoops / 2);
      VelocityFactor = (VelocityLoop * 0.5 / AccelerationLoops) / 2;
    }
    else {
      AccelerationLoops = 70;

      VelocityFactor = VelocityLoop * 0.5 / AccelerationLoops;
    }
    TotalLoops = DeltaPosition / VelocityLoop + AccelerationLoops;
    EndOne = (VelocityFactor * pow(AccelerationLoops, 2));
  }
  else if (Iteration >= 0 && Iteration <= AccelerationLoops) {
    if (StartPosition < SetPosition) {
      Setpoint = VelocityFactor * pow(Iteration, 2) + StartPosition;
    }
    else {
      Setpoint = -VelocityFactor * pow(Iteration, 2) + StartPosition;
    }
  }
  else if (Iteration > AccelerationLoops && Iteration <= (TotalLoops - AccelerationLoops)) {
    if (StartPosition < SetPosition) {
      Setpoint = VelocityLoop * Iteration - EndOne + StartPosition;
    }
    else {
      Setpoint = -VelocityLoop * Iteration + EndOne + StartPosition;
    }
  }
  else if (Iteration >= (TotalLoops - AccelerationLoops) && Iteration <= TotalLoops) {
    if (StartPosition < SetPosition) {
      Setpoint = -VelocityFactor * pow((TotalLoops - Iteration), 2) + DeltaPosition + StartPosition;
    }
    else {
      Setpoint = VelocityFactor * pow((TotalLoops - Iteration), 2) - DeltaPosition + StartPosition;
    }
  }
  else {
    Setpoint = Setpoint;
  }
  return(Setpoint);
}

void setup ()
  {
   ProgFormula ();
  }  // end of setup

void loop ()
  {

  }  // end of loop

Disassembling this I got the following assembler:

000000a8 <_Z11ProgFormulav>:
  a8:   6f 92           push    r6
  aa:   7f 92           push    r7
  ac:   8f 92           push    r8
  ae:   9f 92           push    r9
  b0:   af 92           push    r10
  b2:   bf 92           push    r11
  b4:   cf 92           push    r12
  b6:   df 92           push    r13
  b8:   ef 92           push    r14
  ba:   ff 92           push    r15
  bc:   0f 93           push    r16
  be:   1f 93           push    r17
  c0:   cf 93           push    r28
  c2:   df 93           push    r29
  c4:   00 91 0c 01     lds r16, 0x010C
  c8:   10 91 0d 01     lds r17, 0x010D
  cc:   8f ef           ldi r24, 0xFF   ; 255
  ce:   0f 3f           cpi r16, 0xFF   ; 255
  d0:   18 07           cpc r17, r24
...
 4ec:   cb 01           movw    r24, r22
 4ee:   df 91           pop r29
 4f0:   cf 91           pop r28
 4f2:   1f 91           pop r17
 4f4:   0f 91           pop r16
 4f6:   ff 90           pop r15
 4f8:   ef 90           pop r14
 4fa:   df 90           pop r13
 4fc:   cf 90           pop r12
 4fe:   bf 90           pop r11
 500:   af 90           pop r10
 502:   9f 90           pop r9
 504:   8f 90           pop r8
 506:   7f 90           pop r7
 508:   6f 90           pop r6
 50a:   08 95           ret

0000050c <setup>:

So that function (ProgFormula) has taken up:

0x50a - 0xa8 = 1122 bytes

Now by removing the "static" declarations, like this:

   int StartPosition;
   int DeltaPosition;
   int LoopTime = 5;
   int AccelerationLoops;
   int TotalLoops;
   int EndOne;

   float VelocityLoop;
   float VelocityFactor;

It now compiles to:

000000a8 <_Z11ProgFormulav>:
  a8:   6f 92           push    r6
  aa:   7f 92           push    r7
  ac:   8f 92           push    r8
  ae:   9f 92           push    r9
  b0:   af 92           push    r10
  b2:   bf 92           push    r11
  b4:   cf 92           push    r12
  b6:   df 92           push    r13
  b8:   ef 92           push    r14
  ba:   ff 92           push    r15
  bc:   0f 93           push    r16
  be:   1f 93           push    r17
  c0:   cf 93           push    r28
  c2:   df 93           push    r29
  c4:   c0 91 04 01     lds r28, 0x0104
  c8:   d0 91 05 01     lds r29, 0x0105
  cc:   8f ef           ldi r24, 0xFF   ; 255
  ce:   cf 3f           cpi r28, 0xFF   ; 255
  d0:   d8 07           cpc r29, r24
...
 27e:   cb 01           movw    r24, r22
 280:   df 91           pop r29
 282:   cf 91           pop r28
 284:   1f 91           pop r17
 286:   0f 91           pop r16
 288:   ff 90           pop r15
 28a:   ef 90           pop r14
 28c:   df 90           pop r13
 28e:   cf 90           pop r12
 290:   bf 90           pop r11
 292:   af 90           pop r10
 294:   9f 90           pop r9
 296:   8f 90           pop r8
 298:   7f 90           pop r7
 29a:   6f 90           pop r6
 29c:   08 95           ret

0000029e <setup>:

That is now:

0x29e - 0xa8 = 502 bytes

---

Conclusion

Adding "static" has made the function over twice as long. Presumably it is also slower. After all, those extra instructions have be doing something.

to increase the speed I added some static variables to it.

Your belief that making variables static increases the speed does not seem to be supported.

Answer 2

to increase the speed I added some static variables to it. Is the way I am doing this bad? ... Is it better to avoid statics? If so why?

I use this technique too ... IMHO it is neither good nor bad. In a logger, for instance, all the wall clock time stamp information has to be looked up (perhaps time(0)), and formatted at least once in any given second. I use function static variables to prevent duplicating that effort for the 10's to 100's (or even more on a really fast machine) of log entries that happen in the same second.

I suspect your saying "to increase speed" is what the other answers focus on, and many contributors in SE jump on the 'premature optimization' concept. I think you unintentionally misled them.

Maybe your audience would respond more positively to the term 'memoize' ... this technique avoids some work (when it can).

And, just to be complete - Wikipedia agrees with you:

In computing, memoization is an optimization technique used primarily to speed up computer programs by storing the results of expensive function calls and returning the cached result when the same inputs occur again.

Note 1. I think memoization implies that there is work that need not be duplicated every time through the function (sometimes only initialization) ... thus there often will be an if-clause to skip over the un-necessary work.

Note 2. static is not the only way to hold onto previous values, but is probably somewhat more appropriate for a function when you do not want a class object (to hold the previous values).

Note 3. 'memoize' has impact because it avoids work, and this speeds up the whole system.

Answer 3

Any style recommendation is going to be a bit subjective and conditional, but the biggest problem I see here is that the function can only be running one set of variables at a time. What if you wanted to do two of the task your ProgFormula function is a part of simultaneously? You have no way to switch the state of those variables between calls.

You could make a struct to store the state that your function modifies, that way the calling code could swap out the state between calls. Or you could wrap it in a class so the calling code can make as many instances as it needs and not worry about the data each owns.

As a side note, Premature optimization is the root of all evil. The state you are storing is built with a few subtractions and divisions, but the pow function your loop sometimes calls may be the part dominating its running time.