   [C++] Issue with member function corrupting data member
  Hey.I have two classes that hold a Surface_Cache data member. One has a function Cache that returns a reference to the data member, and one doesn't. The one that doesn't have the function works fine; ie the data member is OK upon and after construction, but the one that does isn't OK. The internal Handle_Manager (Surface_Cache derives from Handle_Manager) has junk values.#include <exception>#include "vld.h"#include "SDL.h"#include "Game_Settings.h"#include "SDL_System.h"#include "Test_Game.h"#include "Wrapper.h"int main(int argc, char* argv[]){try{// Initialise SDL.SDL_System sdl(SDL_INIT_EVERYTHING);// Initialise application settings.Settings::Initialise("settings.xml");/* testing */Wrapper wrapper;Wrapper2 wrapper2;/* end testing */// Initialise game.Test_Game game;// Run game until user exits.game.Run();}catch(const std::exception& std_e){std::cout << std_e.what();}return 0;} class Wrapper{public:Wrapper();~Wrapper();private:Surface_Cache cache;};class Wrapper2{public:Wrapper2();~Wrapper2();Surface_Cache& Cache();private:Surface_Cache cache;};Wrapper::Wrapper(): cache(SDL_Tools::Colours::WHITE){}Wrapper::~Wrapper(){}Wrapper2::Wrapper2(): cache(SDL_Tools::Colours::WHITE){}Wrapper2::~Wrapper2(){}Surface_Cache& Wrapper2::Cache(){return cache;}This is the Surface_Cache:class Surface_Cache : public Handle_Manager<std::string>{public:typedef boost::unordered_map<int, SDL_Surface*> surface_cache;typedef surface_cache::iterator cache_it;Surface_Cache(SDL_Colour default_colour_key);// Returns the surface at hash. Throws if non-existant.SDL_Surface* Surface_At(const Handle& handle);private:// Loads a surface from file_name.SDL_Surface* Load_Surface(const std::string& file_name,SDL_Colour* colour_key);// Loads multiple surfaces from file_name.//void Load_Surfaces(const std::string& file_name);surface_cache cache;SDL_Colour default_colour_key;};Surface_Cache::Surface_Cache(SDL_Colour default_colour_key): default_colour_key(default_colour_key){}The Handle_Manager class:template<typename T>class Handle_Manager;template<typename T>class Handle_Impl{public:Handle_Impl() : id(0), owner(NULL) {}Handle_Impl(int id, Handle_Manager<T>& owner) : id(id), owner(&owner) {}const T& operator*(){// Do I have to ensure that value (optional) is initialised?return (owner->Get(id));}Handle_Impl& operator=(Handle_Impl& rhs){id = rhs.id;owner = rhs.owner;return *this;}int id;Handle_Manager<T>* owner;};template<typename T>class Handle_Manager{public:typedef typename boost::shared_ptr<Handle_Impl<T> > Handle;typedef typename boost::weak_ptr<Handle_Impl<T> > Weak_Handle;Handle_Manager() {}virtual ~Handle_Manager() {}/* Get the handle that corresponds to value (if it exists). */virtual Handle Get(const T& value){ /* Get the handle at the index (hash) of value. */hash_map_it it = hash_table.find(value);if(it != hash_table.end()){Handle handle = handles[it->second].lock();return handle;}/* Handle doesn't exist; create entry for value in handles, values and hash table. */return Add(value);}/* Get the value that corresponds to handle (if it exists). */T Get(const Handle& handle){return *values[handle->id];}protected:// Find the first free index in handles.values and return it.// If there is no free index, let caller know with optional.boost::optional<int> Find_Free_Index(){int i = 0;int size = values.size();while(i < size && values != 0) ++i;if(i < size){ // There is a free index; return it.return boost::optional<int>(i);}// There is no free index; caller has to push_back.return boost::optional<int>();}// Adds a handle for value.// Returns added handle.virtual Handle Add(const T& value){boost::optional<int> index = Find_Free_Index();Handle handle;if(index){ // Free index exists; use it.handle = Handle(new Handle_Impl<T>(*index, *this));handles[*index] = handle;values[*index] = value;hash_table.insert(hash_map_pair(value, handle->id));}else{ // No free index exists; push back.handle = Handle(new Handle_Impl<T>(handles.size(), *this));handles.push_back(handle);values.push_back(value);hash_table.insert(hash_map_pair(value, handle->id));}return handle;}virtual void Clean(){for(unsigned int i = 0; i < values.size(); ++i){if(handles.expired()){ // Handle (shared_ptr) expired; clean its respective entries up.// Clean handle entry.handles.reset();hash_map_it it = hash_table.find(*values);if(it != hash_table.end()){ // Clean hash map entry.hash_table.erase(it);}// Clean value entry.values = boost::optional<T>();}}}// Stores weak handles to allow clean to determine when to destroy the shared_ptr.std::vector<Weak_Handle> handles;// Stores the values of each handle to allow the hash -> value association.std::vector<boost::optional<T> > values;// Stores the value -> hash associations.typedef boost::unordered_map<T, int> hash_map;typedef typename hash_map::iterator hash_map_it;typedef std::pair<T, int> hash_map_pair;hash_map hash_table;};Does anyone know why this would happen? I've had a similar problem here that still isn't solved.Cheers.   
  Sorry nothing wrong stands out to me. Maybe you could try to isolate the problem and provide a working sample exhibiting it. Also, you said "junk values" but showing those values and how you get them, and when, and after doing what, would help too.regards   ;  If you declare the Wrapper2 instance before the Wrapper instance, do the symptoms change?   ;  You have a small mistake here, in the Surface_Cache implementation:SDL_Colour default_colour_key;};Surface_Cache::Surface_Cache(SDL_Colour default_colour_key): default_colour_key(default_colour_key){}The member function and the parameter have the same name. Now, what is going to happen is the compiler will come along, notice that you are initializing default_colour_key, and look at what value you want to initialize it to. Except when it does the name lookup, it's going to find the member default_colour_key instead of the parameter to the function.(Someone check me on this, but AFAIK this is undefined behaviour land.)Renaming the member, the function parameter, or both should fix the problem.[edit] I am incorrect; see below posts.[Edited by - ApochPiQ on January 10, 2009 5:56:14 PM];  For initialiser lists it is perfectly legal to use a parameter with the same name as a member. You do have to be careful however, as a small typo in the parameter name will initialise the member with itself...   ;  Well I'll be damned - it does seem to work (tested in VS2005). I'd still recommend renaming one or both of the variables though to avoid confusion [smile]Mybowlcut - is this the exact code you were running? What's your procedure for debugging, and how are you observing these junk values?