Dear C++ newbies, experts and all of you people in between,

Today, I ended up doing something really stupid, and I don't want any of you doing what I did today. If breakpoints and debugging features are part of your IDE, please, for the love of God, please use them!

While working on a new game concept I thought of a couple of days ago, I stopped and thought to myself "I'm going to use C++ this time and not use pure C to avoid the mistake of letting my cross platform code base getting messy again now that I'm more comfortable with the various mobile and desktop APIs and what not". So I start writing my basic application classes with portability in mind. For reasons I don't feel like getting into, I made my decision to use OpenGL via GLUT on MacOSX. I know, I know, I've never recommended GLUT in the past, but this time, I felt like using it since the game only uses either a mouse or a touch screen anyway (except for the old fashioned exit procedure; the escape key).

I don't know about you, but I have a tendancy to think about how I want my app to execute! I liking having my main() function contain as little code as possible. With that in mind, I ended up writing my main.cpp file to look like this:

game_app_t* app = NULL;

void exit_func()
{
    if( app )
        delete app;
}

//-----------------------------------------------------------------------------
// Name: main
// Desc: Program entry point.
//-----------------------------------------------------------------------------
int main( int argc, char* argv[] )
{
    atexit( exit_func );
    app = new game_app_t;

    return 0;
}

Okay, hold on, I promise you the problem with this code is NOT that obvious! You're probably thinking "Yeah, no s@#% sherlock! You didn't call delete or your game loop function!" Actually, I did. The class constructor calls the initialization function where glutMainLoop is called upon success. Instinctively, I check my code to verify that everything is uninitialized properly, even more than I check for successful initialization. So I put a breakpoint inside of exit_func to verify that it's being called (well, I knew the function works, but I habitually check anyway). Just in case you're wondering, the code for my class (the initialization stuff) looked like this:

#include "game.h"
#include <assert.h>


/* Useful macros */
#define safe_delete(x) if(x) { delete x; x = NULL; }
#define fail_safe(x) if(!x) return;

/* Global class instance */
game_app_t* This = NULL;


/* Default constructor/deconstructor */
game_app_t::game_app_t()
{
    this->init();
}

game_app_t::~game_app_t()
{
    this->uninit();
}

/* Initialization and uninitialization */
bool game_app_t::init()
{
    /* Set global class instance */
    This = this;
    
    /* Initialize OpenGL */
    m_ogldrv = new ogldrv_t();
    assert( m_ogldrv != NULL );
    
    if( !m_ogldrv->init( 0, NULL, 640, 480, GLUT_RGBA | GLUT_DOUBLE | GLUT_DEPTH, "Loop-til" ) )
        return false;
    
    /* Set default callbacks */
    glutDisplayFunc( this->display_func );
    glutIdleFunc( this->idle_func );
    glutKeyboardFunc( this->keyboard_func );
    glutReshapeFunc( this->reshape_func );
    glutMouseFunc( this->mouse_click_func );
    glutPassiveMotionFunc( this->mouse_move_func );
    
    /* Start the main loop */
    glutMainLoop();
    
    return true;
}

void game_app_t::uninit()
{
    safe_delete( m_ogldrv );
}

/* Event callback functions */
void game_app_t::display_func()
{
    fail_safe(This->m_ogldrv);
    
    /* Clear the rendering buffers */
    This->m_ogldrv->clear_buffers();
    
    /* Swap the double buffer */
    This->m_ogldrv->swap_buffers();
}

void game_app_t::idle_func()
{
    glutPostRedisplay();
}

/* Blah blah blah */

And yes, the event callback functions were declared as static, so I needed an extra copy of my this pointer. Since there only needs to be one instance of this class, it doesn't really matter, IMHO.

This is where I found my REAL problem: app is still NULL! I was kinda dumbfounded when I realized that the class instance didn't get uninitialized (hence my deconstructor was never called). I spent a few minutes going over the code to see why this wasn't working. The last thing I wanted was to be leaking memory every launch. After a few more moments of scratching my head, I realized that the problem was that it the call to "new" doesn't return anyway. So the logical thing to do was to use the copy of my this pointer and delete that explicitly instead. After that thought, I added another function:

void game_app_t::exit_func()
{
    safe_delete(This);
}


/* Added after setting all the other callbacks */
atexit( this->exit_func );

So, I added a breakpoint, ran my app in debug mode, and this time, my class instance not only gets deleted properly, but now my deconstructor gets called, and calls the uninit function. Bam, problem solved! Now my main function looks like this:

int main( int argc, char* argv[] )
{
    new game_app_t;

    return 0;
} 

So short, so sweet. Isn't C++ great once you use it properly?

So, the moral of the story is- "Don't be as stupid as Shogun?" Yeah, that too, but please, use your breakpoints and always, ALWAYS, cover your digital butt! I've seen a handful of rookie game programmers who have never learned to use breakpoints (I was one of those). What I did was dumb. I made this mistake for you, please don't do it for yourself.

Shogun.

Wow, you guys are mean!

I have a question. Why are these macros and not methods or even functions?

/* Useful macros */
#define safe_delete(x) if(x) { delete x; x = NULL; }
#define fail_safe(x) if(!x) return;

Does it matter?

Another question. Why are you using

/* Global class instance */
game_app_t* This = NULL;
 

and not

int main( int argc, char* argv[] )
{
    app = new game_app_t();
    /* where game_app_t is an actual class */
    return 0;
}
 

Well, you can't explicitly use "this" in a static function, hence why I created a copy of it. I didn't feel like changing the code structure again if I didn't have to. Second, I forgot to add the '()'.

Mr. Hodgeman, I'm a "heap freak".

Oh, and the macros were just a quick way of getting around. Second, macros can be more optimal (IMO) and for small operations I'd much rather use them to do things that require 2 lines of code than an actual short function. Even if you inline a function that you use frequently, a macro is faster because the compiler doesn't have to push the parameters, call the function, pop them off the stack, do push the registers (eax, ebx, etc.), do whatever, restore the registers and return. I know that it doesn't impact performance really, but after writing and reading so much assembly code, I tend to think about these things a bit too much.

Shogun

template void safe_delete(T* x) { delete x; x = NULL; }

This should probably be safe_delete(T*& x) as otherwise you're only assigning the local variable to NULL.

Wow, are you psychic or something? How did you know I started with MSVC 6.0? Heh, I guess when you're experienced enough, it's just obvious.

As you can see, I'm really old fashioned. No one ever told me that delete NULL was valid, so I'm going to gladly take your advice.

Shogun.