Using static variable in function vs passing variable from caller

Question

I have a function which spawns various types of threads, one of the thread types needs to be spawned every x seconds. I currently have it like this:

bool isTime( Time t )
{
     return t >= now();
}

void spawner()
{
    Time t = now();
    while( 1 )
    {
         if( isTime( t ) )//is time is called in more than one place in the real function
         {
             //launchthread and recalculation of t only happens once in real function
             launchthread()
             t = now() + offset;
         }
    }
}

but I'm thinking of changing it to:

bool isTime()
{
    static Time t = now();
    if( t >= now() )
    {
         t = now() + offset;
         return true;
    }
    return false;
}

void spawner()
{
     while( 1 )
     {
         if( isTime() )
             launchthread();
     }
}

I think the second way is neater but I generally avoid statics in much the same way I avoid global data; anyone have any thoughts on the different styles?

Answer 1

The static Time t approach hides the time from the rest of the code. If this is your intent (you do not need to use a global time somewhere else and the variable t is there just to decide whether to spawn threads or not), the static approach seems neater to me.

However, if your program were for example a simulation, where the current time is crucial to all parts of the program, I would personally go for not hiding the time variable, and implement isTime as in the first piece of code.

Answer 2

One drawback of the static Time t approach is functions with static variables are generally not re-entrant (and are therefore not thread safe) - this may or may not be a problem for you.

Imagine what would happen if you had two independent spawners and used a static Time t.

If you like the second form better, you can achieve something very similar, but without using static:

bool isTime(Time &t)
{
    if( t >= now() )
    {
         t = now() + offset;
         return true;
    }
    return false;
}

void spawner()
{
    Time t = now();
    while (1)
    {
     if( isTime(t) )
          launchthread();
    }
}

Answer 3

Apart from the problem I cited in the comments to the question, you should avoid clever tricks like the plague. The first form (after fixing the bug) is cleaner and easier to understand. The second form, OTOH, confuses the reader by giving the impression that the assignment to t and the test t >= now() happen immediately after each other, who, on realising that its static, then has to try to grok the algorithm a second time.

Also, you can never reset the second function or use it from multiple threads.

Answer 4

The 'second way' has an easier to read spawner function. But it can be made equally readable by using e.g. a member variable i.s.o. a global state.

struct Spawner {
    time when_to_wakemeup;
    timediff step;

    Spawner( timediff astep ): when_to_wakemeup(Now()+astep),step(astep){
    }


    // this is the member-function equivalent of your "spawner" function
    void keep_spawning() {

        while(true) {
            while( Now() < when_to_wakemeup ) Wait();
            when_to_wakemeup += step;
            spawn_one();
        }
     }

     void spawn_one() {
        //... whatever you need
     }
  };

With a class like this, you can create a number of "Spawners" and don't have to bother with guarding your global state:

  Spawner very_often( .5 );
  very_often.keep_spawning();

Answer 5

I recommend to use the first version since it's more testable. The input and output is clearly visible and you can nicely perform boundary tests by feeding funny Time values into it. This is not possible if you have code which references global and/or static data.

Answer 6

If you have a library that offers threads, it should also offer timers. The WinAPI for example offers functionality to call a given function every X seconds. This would probably be the best solution.