views:

279

answers:

8

Suppose I have a free function called InitFoo. I'd like to protect this function from being called multiple times by accident. Without much thought I wrote the following:

void InitFoo()
{
    {
        static bool flag = false;
        if(flag) return;
        flag = true;
    }

    //Actual code goes here.
}

This looks like a big wart, though. InitFoo does not need to preserve any other state information. Can someone suggest a way to accomplish the same goal without the ugliness?

Macros don't count, of course.

+3  A: 

Well, a constructor is only automatically called once. If you create a single instance of this class:

class Foo
{
public:
    Foo(void)
    {
        // do stuff
    }
}

Then //do stuff will only execute once. The only way to execute it twice is to create another instance of the class.

You can prevent this by using a Singleton. In effect, //do stuff can only possibly be called once.

GMan
+1 this is a perfect example of the *right* time to use a singleton.
e.James
+10  A: 

You can do it with some different ugliness:

struct InitFoo
{
     InitFoo()
     {
         // one-time code goes here
     }
};

void Foo()
{
    static InitFoo i;
}

You're still using static, but now you don't need to do your own flag checking - static already puts in a flag and a check for it, so it only constructs i once.

Daniel Earwicker
Don't suppose there's any chance of a comment to accompany the downvote?
Daniel Earwicker
Um. I'm still trying to undo my accidental click. Any hint? <sheepish grin>
sbi
LOL, looks like you figured it out!
Daniel Earwicker
Ah, I just found out. Sorry for the confusion.
sbi
+1  A: 

That is exactly how I'd do it. You could use some function pointer shuffling if you want an alternative:

static void InitFoo_impl()
{
    // Do stuff.

    // Next time InitFoo is called, call abort() instead.
    InitFoo = &abort;
}

void (*InitFoo)() = &InitFoo_impl;
John Kugelman
Can be called twice from multiple threads -- if that's an issue.
Lou Franco
Nifty. One thing, though: names like that __InitFoo, for instance, are reserved for the implementation.
@MeadP: fixed. It was already a static; didn't think it made sense to put it in a namespace.
MSalters
A: 

Do you also need it to be multi-thread safe? Look into the Singleton pattern with double-check locking (which is surprising easy to get wrong).

If you don't want a whole class for this, another simple way is:

In a .cpp (don't declare InitBlah in the .h)

 // don't call this -- called by blahInited initialization
static bool InitBlah() 
{
   // init stuff here
   return true;
}
bool blahInited = InitBlah();

No one can call it outside of this .cpp, and it gets called. Sure, someone could call it in this .cpp -- depends on how much you care that it's impossible vs. inconvenient and documented.

If you care about order or doing it at a specific time, then Singleton is probably for you.

Lou Franco
Best not to roll your own double-checked locking for this purpose. Aside from the fact that on some architectures it fundamentally cannot ever work, even with volatile variables, this is what pthread_once, InitOnceExecuteOnce, etc, are for.
Steve Jessop
+1  A: 

I'd like to protect this function from being called multiple times by accident

To me, this sounds like an issue that will only come up during debugging. If that is the case, I would simply do the following:

void InitFoo()
{
    #ifndef NDEBUG
       static bool onlyCalledOnce = TRUE;
       assert(onlyCalledOnce);
       onlyCalledOnce = FALSE;
    #endif

    ...
}

The purpose of this particular wart is easily discerned just by looking at it, and it will cause a nice, big, flashy assertion failure if a programmer ever makes the mistake of calling InitFoo more than once. It will also completely dissapear in production code. (when NDEBUG is defined).

edit: A quick note on motivation:
Calling an init function more than once is probably a big error. If the end user of this function has mistakenly called it twice, quietly ignoring that mistake is probably not the way to go. If you do not go the assert() route, I would recommend at least dumping a message out to stdout or stderr.

e.James
Good call, but I think you meant #ifndef. ;)
A strange motivation - surely it's more convenient for a module to automatically take care of initing itself if possible? See, for example, std::cout and so on.
Daniel Earwicker
@MeadP: Quite true. Fixed.
e.James
@Earwicker: Absolutely true. The question specifies that `InitFoo` is a "free function", and I answered with that limitation in mind. Kudos (and +1) to you for seeing the bigger issue, and for providing an elegant solution.
e.James
A: 

I do exactly that all the time with situations that need that one-time-only-but-not-worth-making-a-whole-class-for. Of course, it assumes you don't worry about thread-related issues. I usually prefix the variable name with "s_" to make it clear that it's a static variable.

Jim Buck
A: 

Hmmm... if you don't object to using Boost, then have a look at boost::call_once:

namespace { boost::once_flag foo_init_flag = BOOST_ONCE_INIT; }

void InitFoo() {
    // do stuff here
}

void FooCaller() {
    boost::call_once(&foo_init_flag, InitFoo);
    // InitFoo has been called exactly once!
}

void AnotherFooCaller() {
    boost::call_once(&foo_init_flag, InitFoo);
    // InitFoo has been called exactly once!
}
D.Shawley
A: 

Not that I am very excited about it, but this is just another way: function object.

#import <iostream>

class CallOnce {
private:
    bool called;
public:
    CallOnce() {
        called = false;
    }
    void operator()(void) {
        if (called) {
            std::cout << "too many times, pal" <<std::endl;
            return;
        }
        std::cout << "I was called!" << std::endl;
        called = true;
    }

};

int main(void) {
    CallOnce call;

    call();
    call();
}
Stefano Borini