Skip to main content
added 3 characters in body
Source Link
Jamal
  • 34.9k
  • 13
  • 133
  • 237

I have a basic cache set up where whenever. Whenever a user requests a bitmap, it fetches it 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, but there's. There's no way to delete it without having the user expressly call a Release method but this(which seems like a bad idea). They are forced not only to release each individual bitmap, but also the cache as well even though they never created it. 

Should I create Initialize and Release methods strictly to new and `deletedelete the std::mapstd::map?

BitmapCache.hBitmapCache.h

BitmapCache.cppBitmapCache.cpp

I have a basic cache set up where whenever a user requests a bitmap it fetches it 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, the cache is created on first use, but there's no way to delete it without having the user expressly call a Release method but this seems like a bad idea. They are forced not only to release each individual bitmap but the cache as well even though they never created it. Should I create Initialize and Release methods strictly to new and `delete the std::map?

BitmapCache.h

BitmapCache.cpp

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?

BitmapCache.h

BitmapCache.cpp

edited tags; edited title
Link
Jamal
  • 34.9k
  • 13
  • 133
  • 237

C++ Static Classclass member destruction in C++

Tweeted twitter.com/#!/StackCodeReview/status/103400712179949568
formatted code, added more tags
Source Link
Casey
  • 748
  • 6
  • 18
#ifndef CBITMAPCACHE_H
#define CBITMAPCACHE_H

#include <allegro\file.h>
#include <allegro\gfx.h>
#include <allegro\draw.h>
#include <allegro\datafile.h>
#include <allegro\color.h>

#include <map>
#include <string>

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<std::string, BITMAP*>* _pool;
    static void CleanCache();
};

#endif
 

 #include "CBitmapCache"CBitmapCache.h"h"

#include <algorithm>

std::map*map<std::string, BITMAP*>* 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]&filename[0]);

    //Create pool on first use.
    if(_pool == NULL) _pool = new std::mapmap<std::string, BITMAP*>();

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

    //Search for requested BITMAP.
    std::mapmap<std::string, BITMAP*>::iterator _iter = _pool->find>find(filename);

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

    //Otherwise, create it, store it, then return it.
    BITMAP* result = load_bmp(filename.c_str(), NULL);
    if(result == NULL) return NULL;
    _pool->insert>insert(std::pairpair<std::string, BITMAP*>(filename, result));
    return result;
}

BITMAP* BitmapCache::GetBitmap(BITMAP* file) {
    if(file == NULL) return NULL;
    if(_pool == NULL) new std::mapmap<std::string, BITMAP*>();

    CleanCache();

    for(std::mapmap<std::string, BITMAP*>::iterator _iter = _pool->begin>begin(); _iter != _pool->end>end(); ++_iter) {
        if(_iter->second>second != file) continue;
        return _iter->second;>second;
    }
    return NULL;
}

std::string BitmapCache::GetBitmapFilename(BITMAP* file) {
    if(file == NULL) return std::string("""");
    if(_pool == NULL) return std::string("""");
    CleanCache();
    for(std::mapmap<std::string, BITMAP*>::iterator _iter = _pool->begin>begin(); _iter != _pool->end>end(); ++_iter) {
        if(_iter->second>second != file) continue;
        return _iter->first;>first;
    }
    return std::string("""");
}

BITMAP* BitmapCache::GetBlankBitmap(int width, int height) {

    //Smallest allowed size is 1x1.
    if(width < 1 || height < 1) return NULL;

    //Create pool on first use.
    if(_pool == NULL) _pool = new std::map<std::string, BITMAP*>();

    //Cleans the cache.
    CleanCache();

    //Partial Sequential Search for requested BITMAP.
    if(_pool->empty>empty() == false) {
        for(std::mapmap<std::string, BITMAP*>::iterator _iter = _pool->begin>begin(); _iter != _pool->end>end(); ++_iter) {
            //String keys sorted in ascending order.
            //If key is not empty reached non-blank section.
            if(_iter->first>first.empty() == false) break;
            if(width == _iter->second>second->w>w &&&& height == _iter->second>second->h>h) {
                return _iter->second;>second;
            }
        }
    }
    //Attempt to create bitmap, if failed, return NULL.
    BITMAP* result = create_bitmap(width, height);
    if(result == NULL) return NULL;

    //Clear to black, store it, then return to caller.
    clear_bitmap(result);
    _pool->insert>insert(std::pairpair<std::string, BITMAP*>("""", result));
    return result;
}

void BitmapCache::CleanCache() {
    //Clean the pool of any NULL bitmaps that were deleted by caller.
    for(std::mapmap<std::string, BITMAP*>::iterator _iter = _pool->begin>begin(); _iter != _pool->end>end(); ++_iter) {
        if(_iter->second>second != NULL) continue;
        _pool->erase>erase(_iter);
    }
}
 
 
#ifndef CBITMAPCACHE_H
#define CBITMAPCACHE_H

#include <allegro\file.h>
#include <allegro\gfx.h>
#include <allegro\draw.h>
#include <allegro\datafile.h>
#include <allegro\color.h>

#include <map>
#include <string>

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<std::string, BITMAP*>* _pool;
static void CleanCache();
};

#endif
 

 #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 _iter->second;

    //Otherwise, create it, store it, then return it.
    BITMAP* result = load_bmp(filename.c_str(), NULL);
    if(result == NULL) return NULL;
    _pool->insert(std::pair(filename, result));
    return result;
}

BITMAP* BitmapCache::GetBitmap(BITMAP* file) {
    if(file == NULL) return NULL;
    if(_pool == NULL) new std::map();

    CleanCache();

    for(std::map::iterator _iter = _pool->begin(); _iter != _pool->end(); ++_iter) {
        if(_iter->second != file) continue;
        return _iter->second;
    }
    return NULL;
}

std::string BitmapCache::GetBitmapFilename(BITMAP* file) {
    if(file == NULL) return std::string("");
    if(_pool == NULL) return std::string("");
    CleanCache();
    for(std::map::iterator _iter = _pool->begin(); _iter != _pool->end(); ++_iter) {
        if(_iter->second != file) continue;
        return _iter->first;
    }
    return std::string("");
}

BITMAP* BitmapCache::GetBlankBitmap(int width, int height) {

    //Smallest allowed size is 1x1.
    if(width ();

    //Cleans the cache.
    CleanCache();

    //Partial Sequential Search for requested BITMAP.
    if(_pool->empty() == false) {
        for(std::map::iterator _iter = _pool->begin(); _iter != _pool->end(); ++_iter) {
            //String keys sorted in ascending order.
            //If key is not empty reached non-blank section.
            if(_iter->first.empty() == false) break;
            if(width == _iter->second->w && height == _iter->second->h) {
                return _iter->second;
            }
        }
    }
    //Attempt to create bitmap, if failed, return NULL.
    BITMAP* result = create_bitmap(width, height);
    if(result == NULL) return NULL;

    //Clear to black, store it, then return to caller.
    clear_bitmap(result);
    _pool->insert(std::pair("", result));
    return result;
}

void BitmapCache::CleanCache() {
    //Clean the pool of any NULL bitmaps that were deleted by caller.
    for(std::map::iterator _iter = _pool->begin(); _iter != _pool->end(); ++_iter) {
        if(_iter->second != NULL) continue;
        _pool->erase(_iter);
    }
}
 
 
#ifndef CBITMAPCACHE_H
#define CBITMAPCACHE_H

#include <allegro\file.h>
#include <allegro\gfx.h>
#include <allegro\draw.h>
#include <allegro\datafile.h>
#include <allegro\color.h>

#include <map>
#include <string>

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<std::string, BITMAP*>* _pool;
    static void CleanCache();
};

#endif
#include "CBitmapCache.h"

#include <algorithm>

std::map<std::string, BITMAP*>* 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<std::string, BITMAP*>();

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

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

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

    //Otherwise, create it, store it, then return it.
    BITMAP* result = load_bmp(filename.c_str(), NULL);
    if(result == NULL) return NULL;
    _pool->insert(std::pair<std::string, BITMAP*>(filename, result));
    return result;
}

BITMAP* BitmapCache::GetBitmap(BITMAP* file) {
    if(file == NULL) return NULL;
    if(_pool == NULL) new std::map<std::string, BITMAP*>();

    CleanCache();

    for(std::map<std::string, BITMAP*>::iterator _iter = _pool->begin(); _iter != _pool->end(); ++_iter) {
        if(_iter->second != file) continue;
        return _iter->second;
    }
    return NULL;
}

std::string BitmapCache::GetBitmapFilename(BITMAP* file) {
    if(file == NULL) return std::string("");
    if(_pool == NULL) return std::string("");
    CleanCache();
    for(std::map<std::string, BITMAP*>::iterator _iter = _pool->begin(); _iter != _pool->end(); ++_iter) {
        if(_iter->second != file) continue;
        return _iter->first;
    }
    return std::string("");
}

BITMAP* BitmapCache::GetBlankBitmap(int width, int height) {

    //Smallest allowed size is 1x1.
    if(width < 1 || height < 1) return NULL;

    //Create pool on first use.
    if(_pool == NULL) _pool = new std::map<std::string, BITMAP*>();

    //Cleans the cache.
    CleanCache();

    //Partial Sequential Search for requested BITMAP.
    if(_pool->empty() == false) {
        for(std::map<std::string, BITMAP*>::iterator _iter = _pool->begin(); _iter != _pool->end(); ++_iter) {
            //String keys sorted in ascending order.
            //If key is not empty reached non-blank section.
            if(_iter->first.empty() == false) break;
            if(width == _iter->second->w && height == _iter->second->h) {
                return _iter->second;
            }
        }
    }
    //Attempt to create bitmap, if failed, return NULL.
    BITMAP* result = create_bitmap(width, height);
    if(result == NULL) return NULL;

    //Clear to black, store it, then return to caller.
    clear_bitmap(result);
    _pool->insert(std::pair<std::string, BITMAP*>("", result));
    return result;
}

void BitmapCache::CleanCache() {
    //Clean the pool of any NULL bitmaps that were deleted by caller.
    for(std::map<std::string, BITMAP*>::iterator _iter = _pool->begin(); _iter != _pool->end(); ++_iter) {
        if(_iter->second != NULL) continue;
        _pool->erase(_iter);
    }
}
Source Link
Casey
  • 748
  • 6
  • 18
Loading