Sprite cache for SFML sprites

I'm trying to make a sprite cache for SFML sprites. The basic idea I got was to use a map of an enum and a sprite pointer. Then when I would ask for a certain sprite I'd have a manager that would check if the pointer for that sprite is null. If it is, then it would create a new sprite object and clip it to fit what it's supposed to be. If it isn't null, then it means that the sprite object for this particular enum already exists, and I get that.

So far I've made this and it works, but I'm not sure I did it quite right.

This is my example code. It contains a small class called SpriteManager which holds the sprite map. It does the before mentioned pointer checking and creating, as well as assigning a texture to, and clipping the sprite.

Here's the example code:

```
#include
#include
#include

enum Sprites
{
ITEM1,
ITEM2,
ITEM3,
ITEM4,
ITEM5
};

const int WIDTH = 100;
const int HEIGHT = 100;

typedef std::map SpriteMap;
typedef std::map::iterator iter;

class SpriteManager
{
private:
SpriteMap spriteMap;
sf::Texture& m_texture;
void clipSprite(Sprites name)
{
spriteMap.at(name)->setTexture(m_texture);
switch(name)
{
case ITEM1: spriteMap.at(name)->setTextureRect(sf::IntRect(0,0,WIDTH,HEIGHT));break;
case ITEM2: spriteMap.at(name)->setTextureRect(sf::IntRect((1*WIDTH),0,WIDTH,HEIGHT));break;
case ITEM3: spriteMap.at(name)->setTextureRect(sf::IntRect((2*WIDTH),0,WIDTH,HEIGHT));break;
case ITEM4: spriteMap.at(name)->setTextureRect(sf::IntRect((3*WIDTH),0,WIDTH,HEIGHT));break;
case ITEM5: spriteMap.at(name)->setTextureRect(sf::IntRect((4*WIDTH),0,WIDTH,HEIGHT));break;
}
}
public:
SpriteManager(sf::Texture& texture) : m_texture(texture)
{}
sf::Sprite* getSprite(Sprites name)
{
if(!spriteMap[name])
{
spriteMap[name] = new sf::Sprite();
clipSprite(name);
return spriteMap[name];

The code looks good, and your question is well written: thanks.

-
I would call the Sprites enum SpriteId since at any given point it only store one sprite id.

-
Returning a pointer could be quite problematic here: as you say, if SpriteManager goes out of scope then any reference to those sprites will cause issues. The simplest thing to do is to use a shared_ptr: it won't be deleted until no one references it.

-
There is no default case in your switch and your names are not descriptive.

-
The number and position of sprites is hardcoded and isn't easy to change. Did you consider using a string as a key, and storing the sprite coordinates in a file elsewhere?

EDIT answer:

• When the enum is not found, you certainly don't want to fail silently, and the way you fail depends on your objectives. You can have a production mode where you don't fail, but otherwise you want this error to be as visible as possible: throwing an exception/ending the program is a good idea.

• You no longer need a destructor indeed, but you still need to think about potential child classes: if you want to allow inheritance, then add an empty virtual constructor. Another thing to consider is that you'll need this code when you'll be hunting for memory leaks: a tool like valgrind will tell you that there are still references to those pointers only if they get reset in SpriteManager.

• I think you need to use typedef typename instead of just typedef.

StackExchange Code Review Q#21345, answer score: 2