   review my settings manager class
  Getting into the heavy parts of my game engine, I just thought I'd submit some of the underlying systems that it uses to be ripped apart by the GameDev community.  This SettingsManager is used to save/load settings, so for example the user could resize a window, whose new size/position are then saved for reuse the next time the app is opened.  Using strings, the settings data is human readable, and the entire file can be deleted without affecting the engine at all.   class Setting {public:    Setting::Setting( std::wstring const& );    Setting::Setting( Setting const& );    Setting::~Setting();public:    Setting const& Setting::operator = ( Setting const& );    template< typename T >   friend void operator << ( Setting&, T const& );    template< typename T >   friend void operator >> ( Setting const&, T& );private:    std::wstring Setting::Name;    std::wstring Setting::Parameters;};template< typename T >void operator << ( Setting& Storage, T const& User ) {std::wstringstream wss;wss << User; Storage.Parameters = wss.str();}template< typename T >void operator >> ( Setting const& Storage, T& User ) {std::wstringstream wss( Storage.Parameters );wss >> User;}class SettingsManager : public Singleton<SettingsManager> {public:    SettingsManager::SettingsManager();    SettingsManager::SettingsManager( SettingsManager const& );    SettingsManager::~SettingsManager();    int SettingsManager::Initialize( std::wstring const& Filename, bool OverwriteOld = true );    int SettingsManager::Shutdown();public:    SettingsManager const& SettingsManager::operator = ( SettingsManager const& );    // load file data from a stream    int SettingsManager::Load( std::wistream&, bool OverwriteOld = true );    int SettingsManager::Save( std::wostream& );    // clear loaded settings    int SettingsManager::Flush();private:    // last used filename    std::wstring SettingsManager::LastFilename;    // let settings class access this    friend class Setting;    // setting name - value map    typedef std::map< std::wstring, std::wstring > SettingsManager::SettingsMap;    SettingsManager::SettingsMap Settings;}; #include "utility/settings.h"#include <fstream>Setting::Setting( std::wstring const& name ) : Name( name ) {// load setting data from SettingsManagerSettingsManager::SettingsMap::iterator it;it = SettingsManager::GetSingleton().Settings.find( this->Name );if( it != SettingsManager::GetSingleton().Settings.end() )this->Parameters = it->second;}// copy constructorSetting::Setting( Setting const& rhs ) :  Name( rhs.Name ), Parameters( rhs.Parameters ) {}// destructorSetting::~Setting() {// save settingif( this->Parameters.length() > 0 )SettingsManager::GetSingleton().Settings[Name] = Parameters;}// assignment operatorSetting const& Setting::operator = ( Setting const& rhs ) {this->Name = rhs.Name;this->Parameters = rhs.Parameters;return *this;}SettingsManager::SettingsManager() {}SettingsManager::SettingsManager( SettingsManager const& rhs ) : Settings( rhs.Settings ) {}SettingsManager::~SettingsManager() {this->Shutdown();}int SettingsManager::Initialize( std::wstring const& WideFilename, bool Overwrite ) {// copy parameterSettingsManager::LastFilename = WideFilename;// convert to narrow namechar Filename[512];::wcstombs( Filename, WideFilename.c_str(), 511 );// open settings filestd::wifstream FileStream;FileStream.open( Filename );if( !FileStream.good() ) return -1;// load settingsint r = SettingsManager::Load( FileStream, Overwrite );// close fileFileStream.close();return r;}// shutdownint SettingsManager::Shutdown() {// check if we have a name to convertif( SettingsManager::LastFilename.length() <= 0 ) return -1;// convert name to narrowchar NarrowName[512];::wcstombs( NarrowName, SettingsManager::LastFilename.c_str(), 511 );// open file for savingstd::wofstream FileStream;FileStream.open( NarrowName, std::ios::trunc );if( !FileStream.good() ) return -1;// save settingsSettingsManager::Save( FileStream );// close fileFileStream.close();// clean up membersSettingsManager::Flush();return 0;}// assignment operatorSettingsManager const& SettingsManager::operator = ( SettingsManager const& rhs ) {this->Settings = rhs.Settings;return *this;}// load settings from fileint SettingsManager::Load( std::wistream& in, bool Overwrite ) {// while stream isn't finishedwhile( !in.eof() ) {// get current line into bufferstd::wstring Buffer;std::getline( in, Buffer );// translate bufferint nPos = Buffer.find_first_of( L'=' );std::wstring Command = Buffer.substr( 0, nPos );std::wstring Parameter = Buffer.substr( nPos + 1 );// insert buffer into SettingsMapif( ( Command.length() > 0 ) && ( Parameter.length() > 0 ) ) {// if overwrite flag is clearedif( Overwrite == false ) {// check if entry already existsSettingsManager::SettingsMap::iterator it;it = SettingsManager::Settings.find( Command );if( it != SettingsManager::Settings.end() )it->second = Parameter;else SettingsManager::Settings.insert( std::make_pair( Command, Parameter ) );}else SettingsManager::Settings.insert( std::make_pair( Command, Parameter ) );}}return 0;}// save settings to fileint SettingsManager::Save( std::wostream& out ) {SettingsMap::iterator it  = SettingsManager::Settings.begin();for( ; it != SettingsManager::Settings.end(); it++ )out << it->first << L"=" << it->second << std::endl;return 0;}int SettingsManager::Flush() {if( this != 0 ) SettingsManager::Settings.clear();return 0;}SettingsManager::Create();SettingsManager::GetSingleton().Initialize( L"settings.ini" );long SavedValue = 0xdeadbeef;Setting( L"SavedValue" ) << 0xc0de;Setting( L"SavedValue" ) >> SavedValue;SettingsManager::GetSingleton().Shutdown();SettingsManager::Destroy();  The file settings.ini now has the line "SavedValue=49734" (0xc0de in base ten), and the long SavedValue has the value 0xc0de.- psykredit: Ah hell, I might as well post the source code in case anyone is interested.[edited by - psykr on February 15, 2004 8:24:59 PM]
  I like it...got a ring to it.[/sarge]I''d use monostate over singleton here since you have explicit setup/teardown functions. Having a copy constructor doesn''t make sense to me, either.I''m not sure I like the syntax for modifying individual settings, however. I''m considering something like the following: Preferences prefs(L"prefs.ini");if(! prefs.IsLoaded()){//set defaultsprefs["WindowWidth"] = 54;prefs["PlayerName"] = L"antareus is cool and so are you for reading this";};  The copy constructor / operator = are there and unimplemented in case some programmer somewhere does something silly.  I try to have my code work to prevent errors on the programmer''s part (prevent code misuse?).In the beginning stages of the class, I was planning on potentially having multiple SettingsManager instances; one with loaded values, another with defaults.. I guess that since the class''s usage evolved, I haven''t changed the design to keep up with it.As for the syntax.. the whole idea came to me when I realized that my settings loading code tended to take up too much space:if( LPCTSTR strFullscreen = g_SettingsManager.GetSetting( _T( "graphics_fullscreen" ) ) ) {    if( !_tcsncmp( strFullscreen, _T( "true" ) ) ) {   bFullscreen = true;    }    else if( !_tcsncmp( strFullscreen, _T( "false" ) ) ) {   bFullscreen = false;    }}  Such horrible code as that makes me tend to shy away from checking errors excessively, so instead of checking if( !prefs.IsLoaded() ), I simply return default acceptable behavior (not loaded from file). ;  i was wondering something, in your Initialize and Shutdown functions you de-wide the string to a static array, and i guess in theory you wont over flow a 512char path name, but i was wondering why do it that way? Why not a dynamic array or a vector&ltchar&gt resized to the length of the filename? ;  Things that I see that are missing:- categories within the file, for logical grouping- allowing multiple instances of a key (yes this means you can't use std::map anymore). Closest you'll get to storing an array inside an INI file. And a variable length one at that.- ignoring comments within the file, as well as a way to programatically insert them- specifying defaults for values read in, and I don't just mean "" for strings and 0 for integers. I mean 800 for screen width, "Player" for playername, etc. That way if the user deletes the file or only certain lines within the file, it can be reconstructed piece by piece with defaults as the game runs.- support for reading in native data types without a needless abstraction layer (Setting). It's nice to be able to do this:  manager.ReadInt(screen_width);   straight up.These things make your code more complex, but they give a lot more functionality.And last, I would really avoid singleton or monostate or any masquerading-global pattern. You will very likely want more than one settings file for an ENTIRE GAME. [edited by - foofightr on February 15, 2004 11:53:52 PM];  _the_phantom_: The reason that I have to convert at all is because file streams can only be opened with a narrow name, and I thought that passing a wide name, then converting it makes my application flow better with wide characters.  I could pass a narrow string as the parameter, but I decided against it.quote:Original post by foofightr  - categories within the file, for logical groupingI got around the problem of two settings with the same name (dimensions for the window and the graphics device) by prefixing the subsystem name on, but eventually I''ll write a full-fledged INI parser.quote:- allowing multiple instances of a key (yes this means you can''t use std::map anymore). Closest you''ll get to storing an array inside an INI file. And a variable length one at that.I''ve never heard of arrays being used in an INI file before, what would be an example of when I use them?quote:- ignoring comments within the file, as well as a way to programatically insert themAssuming that you mean ignoring characters on a line after a semicolon, that would be easy to read, but I''d have to completely redesign my INI reader to write them back in the same way.  I''ll have to redo it eventually, though.quote:- specifying defaults for values read in, and I don''t just mean "" for strings and 0 for integers. I mean 800 for screen width, "Player" for playername, etc. That way if the user deletes the file or only certain lines within the file, it can be reconstructed piece by piece with defaults as the game runs.I have defaults specified by the client (the subsystem using the SettingsManager). For instance, a variable Width is initialize to 800 (the default value), then is passed to a Setting object for modification, if needed.quote:- support for reading in native data types without a needless abstraction layer (Setting). It''s nice to be able to do this:  manager.ReadInt(screen_width);    straight up.I suppose it''s just a matter of taste, but I don''t want to require access to a main SettingsManager object across all files.  Wrapping it in an individual helper class that handles an uncreated object was just a style choice for me.quote:And last, I would really avoid singleton or monostate or any masquerading-global pattern. You will very likely want more than one settings file for an ENTIRE GAME.I was thinking about this when I designed the class, so I used a singleton pattern instead of monostate.  The way my singleton-ness is implemented, I can remove the inheritance and implement a static function to get the current settings file or something.  But what would be an example when I need more than one settings file? ;quote:Original post by psykr  _the_phantom_: The reason that I have to convert at all is because file streams can only be opened with a narrow name, and I thought that passing a wide name, then converting it makes my application flow better with wide characters.  I could pass a narrow string as the parameter, but I decided against it.yeah, i followed that, i just wondered why the static array and not a dynamic one?;  Oh, good point about using a dynamic array.  I guess I just hadn''t thought about it, there''s no reason I shouldn''t. ;  shouldnt the constructors be private if its a singelton pattern? even if you have overloaded new and inherited from it a copy could be created on the stack with public constructors no? ;  It''s not really a singleton the way most singletons are implemented; it just has the access patterns of a singleton.  Despite the fact that I try to make my code prevent careless errors on the programmer''s part, this is one case where I thought that I''d add in full singleton-ness when I''ve finalized the design. 