To follow on from my last point, we need to control the lifetime of the objects in the map. This is especially important because the map is global, so it will be destroyed after main() ends. If you call SDL_Quit() during main, it is undefined whether you can safely call SDL_FreeSurface() on the pointers in the map.
This is one good reason to not use a global for this.
If we ignore that aspect for the moment, one way to achieve the cleaning up of the surfaces is to use smart pointers. Smart pointers try to automatically clean up after they are done. They are fairly clever, but you can confuse them in certain circumstances that won't arise in the context of this (if you're curious, they don't work with
cyclical references).
If your compiler supports it, you can use the standard smart pointers std::shared_ptr or std::weak_ptr in the map. If your compiler doesn't support these new classes, consider upgrading! If you cannot or will not upgrade, the
boost libraries can provide these classes for you.
Shared pointer is a really nice class that manages shared ownership of an interesting object. When the last owner object that needs an object is finished using it, the shared pointer will cleanup the owned object. The clients would store a shared_ptr. You can specify SDL_FreeSurface() as the "cleanup" function when you create the smart pointer. This is important, because shared_ptr defaults to trying to
delete the object which was passed, which is incorrect for objects that were not allocated with
new.
Something like this:
#include <map>
#include <string>
#include <memory>
#include "SDL.h"
#include "SDL_image.h"
typedef std::shared_ptr<SDL_Surface> SurfacePtr;
typedef std::map<std::string, SurfacePtr> SurfaceCache;
SurfaceCache surfaceCache;
SurfacePtr load(const std::string &name)
{
SurfaceCache::iterator i = surfaceCache.find(name);
if(i != surfaceCache.end())
{
return i->second;
}
SDL_Surface *loaded = IMG_Load(name.c_str());
if(loaded == NULL)
{
// TODO: handle this here?
}
SurfacePtr surface(loaded, &SDL_FreeSurface);
surfaceCache.insert(std::make_pair(name, surface));
return surface;
}
Note this code shows a reasonably idiomatic search for and add entries to a std::map. It avoids extra, unnecessary lookups in the best case. In constrast, BeerNutts's simpler code performs at least two, and worst case three searches for an element.
But we are still left with the situation I mentioned earlier, where the map is destroyed after main. In general, running complicated code like constructors and destructors outside main isn't a good idea. You cannot depend on exactly when these functions will be called relative to the rest of your program. If you aren't careful, it is easy to trigger undefined behaviour.
One solution is to manage the lifetime of the global in main(). main() creates the global and exports it in a controlled manner. Something like the following:
// image.h
#ifndef IMAGE_H
#define IMAGE_H
#include <map>
#include <string>
#include <memory>
#include "SDL.h"
#include "SDL_image.h"
typedef std::shared_ptr<SDL_Surface> SurfacePtr;
typedef std::map<std::string, SurfacePtr> SurfaceCache;
SurfacePtr load(const std::string &name);
#endif
// image.cpp
#include "image.h"
SurfaceCache &globalSurfaceCache();
SurfacePtr load(const std::string &name)
{
SurfaceCache &surfaceCache = globalSurfaceCache();
SurfaceCache::iterator i = surfaceCache.find(name);
if(i != surfaceCache.end())
{
return i->second;
}
SDL_Surface *loaded = IMG_Load(name.c_str());
if(loaded == NULL)
{
// TODO: handle this here?
}
SurfacePtr surface(loaded, &SDL_FreeSurface);
surfaceCache.insert(std::make_pair(name, surface));
return surface;
}
main.cpp
#include <iostream>
#include <cstdlib>
#include "image.h"
namespace {
static SurfaceCache *hiddenSurfaceCache = 0;
}
SurfaceCache &globalSurfaceCache() {
return *hiddenSurfaceCache;
}
int main(int, char**)
{
if(SDL_Init(SDL_INIT_VIDEO) < 0)
{
std::cerr << "Failed to initialise SDL: " << SDL_GetError() << '\n';
return 1;
}
std::atexit(&SDL_Quit);
SDL_Surface *screen = SDL_SetVideoMode(800, 600, 0, SDL_SWSURFACE);
if(!screen)
{
std::cerr << "Failed to set video mode: " << SDL_GetError() << '\n';
return 1;
}
SurfaceCache surfaceCache;
hiddenSurfaceCache = &surfaceCache;
bool running = true;
while(running)
{
// ...
}
hiddenSurfaceCache = 0;
return 0;
}
This offsets most of the damage of the global. The lifecycle of the cache is automatically controlled in main(), and the pointer is nullified before it is invalidated (by falling off the end of main). I've also hidden the declaration of globalSurfaceCache() inside image.cpp, so client code will not directly interact with it.