HiveBrain v1.2.0
Get Started
← Back to all entries
patterncppMinor

Static class member destruction in C++

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
destructionmemberclassstatic

Problem

I have a basic cache set up. Whenever a user requests a bitmap, it fetches or loads it from disk if it isn't already loaded, significantly reducing load times.

Currently, the design explicitly tells the user that they are responsible for freeing the individual bitmaps when they are done. That's fine, but the biggest problem I notice right away is that the cache is created on first use. There's no way to delete it without having the user expressly call a Release method (which seems like a bad idea). They are forced not only to release each individual bitmap, but also the cache even though they never created it.

Should I create Initialize and Release methods strictly to new and delete the std::map?

(It's named pool in the code because the member name hasn't been updated as of this posting.)

BitmapCache.h

#ifndef CBITMAPCACHE_H
#define CBITMAPCACHE_H

#include 
#include 
#include 
#include 
#include 

#include 
#include 

struct BITMAP;

class BitmapCache {
public:

    static BITMAP* GetBitmap(std::string filename);
    static BITMAP* GetBitmap(BITMAP* file);
    static std::string GetBitmapFilename(BITMAP* file);
    static BITMAP* GetBlankBitmap(int width, int height);

protected:
private:

    static std::map* _pool;
    static void CleanCache();
};

#endif


BitmapCache.cpp

```
#include "CBitmapCache.h"

#include

std::map* BitmapCache::_pool = NULL;

BITMAP* BitmapCache::GetBitmap(std::string filename) {
//Return NULL if a bad filename was passed.
if(filename.empty()) return NULL;
if(exists(filename.c_str()) == false) return NULL;

//Reduce incorrect results by forcing slash equality.
filename = fix_filename_slashes(&filename[0]);

//Create pool on first use.
if(_pool == NULL) _pool = new std::map();

//Clean the pool if it's dirty.
CleanCache();

//Search for requested BITMAP.
std::map::iterator _iter = _pool->find(filename);

//If found, return it.
if(_iter != _pool->end()) return _

Solution

Ahh, the wonderful world of game design - I assume that is what you are doing.
In my oppinion, the three most common solutions are (in order of my preference):

  • Make the cache an object instead of a pointer to an object. This


way it will automatically be destructed correctly when your
application ends. Disadvantage: No real way to shutdown the game and
'reboot' without restarting the app, but unless you are switching
libraries or graphic backends, this is also not really needed.

  • Leave everything as it is, don't worry. I know, the destructor won't


run, but upon application exit the OS will reclaim every bit of
memory anyway. However, DO NOT DO THIS IF there are 'important'
tasks that need to be taken care of during cache destruction, like
(i don't know) writing something back to disk. Note that this is
only viable if you want to discard the complete cache object (not
only its contents) only upon process termination, or else you are
going to have serious memory leaks. Also do not do this if the application needs to continue execution for a long time after the your game has quit.

  • In case neither 1) or 2) are viable, you are probably in the need to shut down your 'game


engine' and start it back up again without terminating your app. In
this case, however you will have some method anyway to 'shut down
everything'/'close engine'/'reinit'/call it whatever you want. Then
simply, this method can take care of releasing your cache.

Note to solution 2: This 'solution' is dirty and if you are a good coder you really should properly destruct all objects even upon application exit, just like I always do. But to be honest, in this particular case you have almost nothing to gain from it. The only thing that I can think of is, if you are using a memory leak detector, properly terminating everything will spare you thousands of false warnings (which IS important IF you are using a leak detector). If you are not sure, forget about option 2) and do it right.

Context

StackExchange Code Review Q#4135, answer score: 4

Revisions (0)

No revisions yet.