(Ab)using constructors and destructors for side effects bad practice? Alternatives?

Question

In OpenGL, one often writes code like this:

glPushMatrix(); // modify the current matrix and use it glPopMatrix(); 

Essentially, the state is changed, then some actions are performed that use the new state, and finally the state is restored.

Now there are two problems here:

1. It's easy to forget to restore the state.
2. If the code in between throws an exception, the state is never restored.

In true object-based programming style, I have written some utility classes to overcome these problems, like so:

struct WithPushedMatrix {     WithPushedMatrix() { glPushMatrix(); }     ~WithPushedMatrix() { glPopMatrix(); } }; 

Now I can simply write my previous example like this:

WithPushedMatrix p; // modify the current matrix and use it 

The exact moment of restoring is determined by the lifetime of p. If an exception is thrown, p's destructor gets called, the state is restored, and life is good.

Still, I'm not entirely happy. Especially if the constructor takes some arguments (e.g. flags for glEnable), it's easy to forget to assign the object to a variable:

WithEnabledFlags(GL_BLEND); // whoops! 

The temporary gets destroyed immediately, and the state change is reversed prematurely.

Another issue is that anyone else reading my code might get confused: "Why is there a variable declared here that is never used? Let's get rid of it!"

So, my questions: Is this a good pattern? Does it maybe even have a name? Are there any problems with this approach that I'm overlooking? Last but not least: are there any good alternatives?

Update: Yes, I guess it's a form of RAII. But not in the way RAII is normally used, because it involves a seemingly useless variable; the "resource" in question is never accessed explicitly. I just didn't realize that this particular usage was so common.

Answer 1

I like the idea of using RAII to control OpenGL state, but I'd actually take it one step further: have your WithFoo class constructor take a function pointer as a parameter, which contains the code you want to execute in that context. Then don't create named variables, and just work with temporaries, passing in the action you want to execute in that context as a lambda. (needs C++0x, of course - can work with regular function pointers too but it's not nearly as pretty.)
Something like this: (edited to restore exception-safety)

class WithPushedMatrix { public:     WithPushedMatrix()     {         glPushMatrix();     }      ~WithPushedMatrix()     {         glPopMatrix();     }      template <typename Func>     void Execute(Func action)     {         action();     } }; 

And use it like so:

WithPushedMatrix().Execute([] {     glBegin(GL_LINES);     //etc. etc. }); 

The temporary object will set up your state, execute the action and then tear it down automatically; you don't have "loose" state variables floating around, and the actions executing under the context become strongly associated with it. You can even nest multiple contextual actions without worrying about destructor order.

You can even take this further and make a generic WithContext class that takes additional setup and teardown function parameters.

edit: Had to move the action() call into a separate Execute function to restore exception-safety - if it's called in the constructor and throws, the destructor won't get called.

edit2: Generic technique -

So I fiddled around with this idea some more, and came up with something better:
I'll define a With class, that creates the context variable and stuffs it into a std::auto_ptr in it's initializer, then calls the action:

template <typename T> class With { public:     template <typename Func>     With(Func action) : context(new T())      { action(); }      template <typename Func, typename Arg>     With(Arg arg, Func action) : context(new T(arg))     { action(); }  private:     const std::auto_ptr<T> context; }; 

Now you can combine it with context type that you defined originally:

struct PushedMatrix  {     PushedMatrix() { glPushMatrix(); }     ~PushedMatrix() { glPopMatrix(); } }; 

And use it like this:

With<PushedMatrix>([] {     glBegin(GL_LINES);     //etc. etc. }); 

or

With<EnabledFlag>(GL_BLEND, [] {     //... }); 

Benefits:

1. Exception-safety is handled by the auto_ptr now, so if action throws, the context will still get destroyed properly.
2. No more need for an Execute method, so it looks clean again! :)
3. Your "context" classes are dead-simple; all of the logic is handled by the With class so you just need to define a simple ctor/dtor for each new type of context.

One niggle: As I've written it above, you need to declare manual overloads for the ctor for as many parameters as you need; although even just one should cover most OpenGL use cases, this isn't really nice. This should be neatly fixed with variadic templates - just replace typename Arg in the ctor with typename ...Args - but it'll depend on compiler support for that (MSVC2010 doesn't have them yet).