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?

The first form has a bug. You should set t once outside the loop. Also, both forms use a busy loop that will completely tie up the CPU. — Marcelo Cantos
– Marcelo Cantos, Commented Jun 3, 2010 at 10:28
Fixed the bug, this is simplified pseudo code, not what I'm actually doing — Patrick
– Patrick, Commented Jun 3, 2010 at 10:38
Self-correction: the second version launches at most one thread, depending on whether the first and second calls to now() fall within the same quantum. It needs the outer while (1) ... loop to match the first version. But I take Patrick's point that it is only pseudo-code. — Marcelo Cantos
– Marcelo Cantos, Commented Jun 3, 2010 at 11:12

Marcelo Cantos · Accepted Answer · 2010-06-03 10:32:50Z

4

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.

answered Jun 3, 2010 at 10:32

Marcelo Cantos

187k40 gold badges338 silver badges366 bronze badges

Sign up to request clarification or add additional context in comments.

2 Comments

Patrick Over a year ago

Someone looking at the isTime function would have to spend a moment working out what is going on, but it is a very simple function so wouldn't take that much time, however someone looking at the more complicated spawner function (which has a lot more going on than shown here) has slightly cleaner code to look at making that easier to grok. 6 of one, half a dozen of the other maybe?

Marcelo Cantos Over a year ago

It is half-and-half, but since the same complexity is present in both, I would much prefer the version that doesn't use a clever trick and is reentrant, to boot.

Suma · Accepted Answer · 2010-06-03 10:44:06Z

3

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();
    }
}

edited Jun 3, 2010 at 10:44

answered Jun 3, 2010 at 10:31

Suma

34.6k19 gold badges131 silver badges202 bronze badges

3 Comments

Patrick Over a year ago

Not a problem when looking at time because time would be the same for both threads - but not a problem here anyway

Patrick Over a year ago

Hmm, I'm not sure, the fact that t changes in isTime is not obvious where isTime is called (although the fact that it won't be a const ref would at least suggest it)...

Suma Over a year ago

If you prefer it to be explicit, you can always change it to pass as a pointer, not reference - the basic idea stays the same.

xtofl · Accepted Answer · 2010-06-03 10:52:33Z

2

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();

edited Jun 3, 2010 at 10:52

answered Jun 3, 2010 at 10:47

xtofl

41.7k13 gold badges112 silver badges203 bronze badges

3 Comments

Patrick Over a year ago

The spawner function is a free function (the main loop) in the code. I think creating a class around it just to make this slightly neater would be overkill...

xtofl Over a year ago

@Patrick: it's up to your design goals. In a stand-alone console app, I would choose the free function. In a library I would go for the class version.

Patrick Over a year ago

Absolutely, I +1'd your answer as it may be useful for someone looking at this thread later, just not what I'm looking for right now

Frerich Raabe · Accepted Answer · 2010-06-03 10:47:41Z

1

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.

answered Jun 3, 2010 at 10:47

Frerich Raabe

95.2k20 gold badges127 silver badges216 bronze badges

Comments

Puppy · Accepted Answer · 2010-06-03 11:08:23Z

1

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.

answered Jun 3, 2010 at 11:08

Puppy

147k40 gold badges271 silver badges480 bronze badges

2 Comments

Patrick Over a year ago

Can't find them on msdn (for c++), can you add a link and is there a boost equivalent?

Puppy Over a year ago

I don't know if Boost provides an equivalent. msdn.microsoft.com/en-us/library/ms686360(v=VS.85).aspx (scroll to bottom for waitable timer reference). You could also check Intel's Thread Building Blocks for similar functionality.

Michał Trybus · Accepted Answer · 2010-06-03 10:26:15Z

0

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.

answered Jun 3, 2010 at 10:26

Michał Trybus

11.9k3 gold badges33 silver badges45 bronze badges

Collectives™ on Stack Overflow

Using static variable in function vs passing variable from caller

6 Answers 6

2 Comments

3 Comments

3 Comments

Comments

2 Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

6 Answers 6

2 Comments

3 Comments

3 Comments

Comments

2 Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related