Replace a #define/template "Variable Wrapper" system with a pure template/OOP solution

Question

I have an inherited code base that I am looking to improve. Below is a "variable wrapper" template class and #defines that i would like to replace with a template only solution, or redesign completely with something else. I have not been able to come up with any solution that would not have to leverage macros or are worse in complexity and clarity than this. I lay it out plainly and hopefully that makes it obvious what this code does.

The code uses hopscotch_map for storage, which is actually irrelevant to the question posed here. It could use std::map just as well.

The goal would be to either have a complete template solution or find a "better way"(tm) to have this functionality using modern design patterns.

This is the #define/template code in it's complete form:

template< class T >
class CfgBase
{
private:
    T m_val;
public:
explicit CfgBase(T t): m_val{t} {}

inline void set(const T t)
{
    m_val = t;
}

inline T get() const
{
    return m_val;
}
};  

void AddConfigData(const string &p, CfgBase< long > *me){
    ConfigVals::m_longMap.insert({p, me});
};
void AddConfigData(const string &p, CfgBase< bool > *me); //these have a similar definition as the long above
void AddConfigData(const string &p, CfgBase< double > *me);
void AddConfigData(const string &p, CfgBase< string > *me);
void AddConfigData(const string &p, CfgBase< unsigned long> *me);

#define CFG_DEF_TYPE( _type, _upper, _key, _dflt )       \
    class CFG_##_upper : public CfgBase< _type >        \
    { public:                  \
    CFG_##_upper(): CfgBase{_dflt} { AddConfigData( _key, this ); }  \
    };                    \
    private:                  \
    CFG_##_upper m_##_upper;              \
    public:                   \
    _type get##_upper() const { return m_##_upper.get(); }     \
    void set##_upper( const _type v ) { m_##_upper.set( v );   }

#define CFG_DECL_STR( _upper, _key, _dflt ) CFG_DEF_TYPE( string, _upper, _key, _dflt )
#define CFG_DECL_DBL( _upper, _key, _dflt ) CFG_DEF_TYPE( double, _upper, _key, _dflt )
#define CFG_DECL_BOOL( _upper, _key, _dflt ) CFG_DEF_TYPE( bool, _upper, _key, _dflt )
#define CFG_DECL_LONG( _upper, _key, _dflt ) CFG_DEF_TYPE( long, _upper, _key, _dflt )
#define CFG_DECL_ULONG( _upper, _key, _dflt ) CFG_DEF_TYPE( unsigned long, _upper, _key, _dflt )

using CfgLongMap = bhopscotch_pg_map< string, CfgBase< long > * >;
using CfgBoolMap = bhopscotch_pg_map< string, CfgBase< bool > * >;
using CfgDblMap = bhopscotch_pg_map< string, CfgBase< double > * >;
using CfgStrMap = bhopscotch_pg_map< string, CfgBase< string > * >;
using CfgULongMap = bhopscotch_pg_map< string, CfgBase< unsigned long > * >;

You then use it in this fashion (only the native "long" type example follows):

class ConfigVals
{
    static CfgLongMap m_longMap;
    CFG_DECL_LONG(SomeCustomVariable, "A_CUSTOM_VARIABLE", 12345)
}

For this example, what happens is the CFG_DECL_LONG macro becomes:

CFG_DEF_TYPE( long, SomeCustomVariable, "A_CUSTOM_VARIABLE", 12345)

which then becomes (remember that macro is contained inside the ConfigVals class):

class CFG_SomeCustomVariable : public CfgBase< long >
{ public:
CFG_SomeCustomVariable(): CfgBase{12345} { AddConfigData("A_CUSTOM_VARIABLE", this ); }
};
private:
CFG_SomeCustomVariable m_SomeCustomVariable;
public:
long getSomeCustomVariable() const { return m_SomeCustomVariable.get(); }
void setSomeCustomVariable( const long v ) { m_SomeCustomVariable.set( v );}

The end result is a class that looks like this:

class ConfigVals
{
    static CfgLongMap m_longMap;
    class CFG_SomeCustomVariable : public CfgBase< long >
    { public:
        CFG_SomeCustomVariable(): CfgBase{12345} { AddConfigData("A_CUSTOM_VARIABLE", this ); }
    };
    private:
    CFG_SomeCustomVariable m_SomeCustomVariable;
    public:
    long getSomeCustomVariable() const { return m_SomeCustomVariable.get(); }
    void setSomeCustomVariable( const long v ) { m_SomeCustomVariable.set( v );}
}

The result is that ConfigVals now has a private variable of CfgBase type that holds the value, that variable is in a lookup map (used elsewhere in the codebase) and ConfigVals has custom function calls that incorporate the variable name.

If you have another variable of type long, it's as simple as adding another macro call to the ConfigVals class. If you want another type, you have to make sure you have coverage by manually adding appropriate code in #defines.

Anyone seen anything like this before and is there a better solution?

EDIT: firda wanted usage elaborated. Here it is.

The ConfigVals class is actually a singleton, it is the centralized source of all configuration variables in a much larger code base:

class ConfigVals{
public:
    ConfigVals();
    virtual ~ConfigVals();

    static ConfigVals& getInstance()
    {
        static ConfigVals instance_; //lazy init on first access, guaranteed creation
        return instance_;
    }
// a whole crapload of macros here
// also a "configuration file" loader function
}

usage is pretty simple, whenever we need to use a particular variable:

#define S_CONFIG ConfigVals::getInstance();
S_CONFIG.getSomeCrucialVariableAtThisMoment();

There is a lot of these everywhere too:

if (S_CONFIG.getSomeStupidVar() >= S_CONFIG.getSomeOtherStupidVar()){
...// do some stuff//
}

On instantiation all variables have default values, adjusted by loading a configuration file. The configuration file loader basically does this for every single type (you have to put an additional one of these for every type you want. it sucks, i know):

auto siter = m_strMap.find(key);
if (siter != m_strMap.cend()) {
    siter->second->set(f1);
    LOG_CONSOLE->debug("CONFIG: {} = {}", key, f1);
    return;
}

the configuration file is in this format:

someValue=2234
someOtherValue=astring
nowaThirdValue=3.4

Answer 1

I am still waiting for justification of that macro-generated CFG_##_upper class, but I would like to address things I have spotted.

CfgBase, indentation, excess inline keyword, get/set v.s. operators

• Not sure if that was just copy-paste error, but you should indent the content of the class (except labels/access which are one-less = same as the class) .
• inline is implicit if you provide inline body (inside class definition, not outside of it), so that can be removed.
• get/set methods should rather be (or add) operator T() and operator=, but...
• I cannot even see the reason for using CfgBase and not the type directly, assuming you do not actually need the macro but just some string-any map.

Per-type maps (ConfigVals::m_typeMap + AddConfigData(...type..))

This should really be generalized, maybe using template with static variable:

template<class T> using ConfigMapImpl = bhopscotch_pg_map<std::string, T>;
template<class T> struct ConfigMapStore { static inline ConfigMapImpl<T> it = {}; };
// template<class T> ConfigMapImpl<T> ConfigMapStore<T>::it = {}; // older standard

• add some templated using (like typedef) of the map implementation to make it easier to swith to different implementation (e.g. = std::map<std::string, T>).
• generalize the maps somehow, I have chosen static variable inside template (see inline variables)
• where is using std::string? Either that somewhere or strictly std::string instead of string everywhere (no using namespace std;!)

Lack of information

The question unfortunatelly lacks a lot of important information / context, so, I have to guess. I imagine that you first create some default configuration (that is what _dflt = default appears to be for - and use full name please), so, you can do so with the above:

ConfigMapStore<long>::it.insert({"some_config_parameter", 1234});

and somewhere later (e.g. parsing of configuration file or maybe commands / communication)

ConfigMapStore<long>::it["some_config_parameter"] = 4321;

Now could also be good time to create custom map that won't create new elements in operator[] but maybe rather throw exception (just change ConfigMapImpl).

---

Update after the edit of the question

Now I realize I have overlooked the true purpose of the macro. It does not only create the class, it also creates methods and private field inside the definition of the class where it is used. I was also wondering about the names and came to conclusion that the only possible reason for named getters would be to somehow merge all the classes into one (and I know it is possible to create variadic-template deriving the result from all passed arguments, because I wrote one), which was very close to the truth - I have just overlooked }; before private: inside that macro. That means that the macro can be removed and pure class/template-solution exists:

template<class T> class ConfigValue {
    T value;
public:
    ConfigValue(std::string name, const T& value): value(value) {
        ConfigMapStore<T>::it.include({name, *this}); }
    // find copy and swap idiom for improvements, I will keep it simple here
    ConfigValue& operator=(const T& rhs) { value = rhs; return *this; }
    operator /*maybe const*/ T&() /*maybe const*/ { return value; }
};
struct Configuration {
    ConfigValue<long> someValue = { "someValue", 1234 };
    // you can define macro for the above, but I would not do so
};

The above would require altering ConfigMapImpl to be some_map<std::string, ConfigValue<T>&>. The reference is important not to make copy ;)

---

Full tested code in VS

#include <string>
#include <unordered_map>
#include <iostream>

template<class T> class ConfigValue;
template<class T> using ConfigMapImpl = std::unordered_map<std::string, ConfigValue<T>&>;
template<class T> struct ConfigMapStore { static inline ConfigMapImpl<T> it = {}; };

template<class T> class ConfigValue {
    T value;
public:
    ConfigValue(std::string name, const T& value) : value(value) {
        ConfigMapStore<T>::it.emplace(name, *this); }
    ConfigValue& operator=(const T& rhs) { value = rhs; return *this; }
    operator const T&() const { return value; }
};
struct Configuration {
    ConfigValue<long> someValue = { "someValue", 1234 };
};

int main()
{
    Configuration cfg;
    std::cout << cfg.someValue << " " << ConfigMapStore<long>::it.at("someValue") << std::endl;
    cfg.someValue = 4321;
    std::cout << cfg.someValue << " " << ConfigMapStore<long>::it.at("someValue") << std::endl;
    std::cin.get();
}

Answer 2

I was thinking about the parsing and printing and came up with a different solution, that uses single map of AbstractValue& (reference because the real value lives inside the configuration) that has abstract print and parse that are defined later in Value<T>. I have also added vector for sequential access, e.g. for generating help or printing all the values or parsing/printing of full configuration.

Same approach (abstract method with override in template) can be used to add other functionality, e.g. something like cfg["port"] = 1234 (which would be a bit more complex and outside of the scope of this review / alternative solution). It currently understands both std::cout << cfg.port << " " << cfg["port"] and std::cin >> cfg["port"]; as can be seen in the test code. The last can be combined with the for (AbstractValue& it : list) seen in printHelp to create full configuration file parsing.

This version of the configuration does not have to be singleton (and of course can, but it does not rely on anything static). The code is for demonstration.

#include <string>
#include <unordered_map>
#include <vector>
#include <functional>
#include <iostream>

/// Holds all configuration values,
/// provides access by name (indexing by string)
/// and enumaration of names (e.g. for automatic help output).
class Configuration {
public:
    /// Abstract value defines all methods needed for a value,
    /// namely print and parse, which are used for streaming
    /// (reading and writing configuration).
    class AbstractValue {
    public:
        /// Name of the variable/property (for lookup and help output)
        const std::string name;
        /// Description of the variable/property (for help)
        const std::string help;

        /// Printing (stream-out) of value
        virtual void print(std::ostream&) const = 0;
        /// Parsing (stream-in) of value
        virtual void parse(std::istream&) = 0;

        /// Printing (stream-out) of value
        friend std::ostream& operator << (std::ostream& s, const AbstractValue& v) { v.print(s); return s; }
        /// Parsing (stream-in) of value
        friend std::istream& operator >> (std::istream& s, AbstractValue& v) { v.parse(s); return s; }
    protected:
        /// Initialization of name, adding to cfg.map and cfg.list
        AbstractValue(Configuration& cfg, std::string name, std::string help) :
            name(std::move(name)), help(std::move(help)) {
            cfg.map.emplace(this->name, *this);
            cfg.list.push_back(*this);
        }
        /// Non copyable
        AbstractValue(const AbstractValue&) = delete;
        /// Non copyable
        AbstractValue& operator=(const AbstractValue&) = delete;
    };
protected:
    // map for access by name
    std::unordered_map<std::string, AbstractValue&> map;
    // list for sequential enumeration (e.g. to generate help page)
    std::vector<std::reference_wrapper<AbstractValue>> list;
public:
    /// Template for conrete configuration values/properties
    template<class T> class Value: public AbstractValue {
        T value;
    public:
        Value(Configuration& cfg, std::string name, const T& value, std::string help) :
            AbstractValue(cfg, std::move(name), std::move(help)), value(value) {}
        Value& operator=(const T& rhs) { value = rhs; return *this; }
        operator const T&() const { return value; }
        void print(std::ostream& s) const override { s << value; }
        void parse(std::istream& s) override { s >> value; }
    };
    /// Access to existing values/properties (uses map.at)
    AbstractValue& operator[](const std::string& name) { return map.at(name); }
    /// Example of enumerating all properties
    void printHelp(std::ostream& s = std::cout) {
        for (const AbstractValue& it : list)
            s << it.name << ": " << it.help << "\n";
        s << std::flush;
    }

public:
    Value<int> port = { *this, "port", 12345, "TCP port of the server" };
};

int main()
{
    Configuration cfg;
    cfg.printHelp();
    std::cout << cfg.port << " " << cfg["port"] << std::endl;
    cfg.port = 54321;
    std::cout << cfg.port << " " << cfg["port"] << std::endl;
    std::cin >> cfg["port"];
    std::cout << cfg.port << " " << cfg["port"] << std::endl;
}

Example output (and input - 1111)

port: TCP port of the server
12345 12345
54321 54321
11111
11111 11111