INI config loader / saver

Question

Please take a review of my simple .ini config loader / saver:

//
// value wrapper
// 
class IniEntry {
public:
    //
    // default ctor
    IniEntry() {

    }

    //
    // copy ctor
    IniEntry(const IniEntry& rhs) : _Data(rhs._Data) {

    }

    //
    // move ctor
    IniEntry(IniEntry&& rhs) noexcept : _Data(std::move(rhs._Data)) {

    }

    //
    // dtor
    ~IniEntry() {

    }

public:
    //
    // copy assignment
    IniEntry& operator=(const IniEntry& rhs) {
        _Data = rhs._Data;
        return *this;
    }

    //
    // move assignment
    IniEntry& operator=(IniEntry&& rhs) noexcept {
        _Data = std::move(rhs._Data);
        return *this;
    }

    //
    // string assignment
    IniEntry& operator=(const char* value) {
        SetAsString(value);
        return *this;
    }

    //
    // integer assignment
    IniEntry& operator=(int value) {
        SetAsInteger(value);
        return *this;
    }

    //
    // float assignment
    IniEntry& operator=(float value) {
        SetAsFloat(value);
        return *this;
    }

public:
    //
    // string explicit cast
    explicit operator const char*() const {
        return GetAsString();
    }

    //
    // integer explicit cast
    explicit operator int() const {
        return GetAsInteger();
    }

    //
    // float explicit cast
    explicit operator float() const {
        return GetAsFloat();
    }

public:
    //
    // set data from string
    inline void SetAsString(const char* value) {
        _Data.assign(value);
    }

    //
    // get data as string
    inline const char* GetAsString(const char* default_return = "") const {
        return _Data.empty() ? default_return : _Data.c_str();
    }

    //
    // set data from integer
    inline void SetAsInteger(int value) {
        _Data.assign(std::to_string(value));
    }

    //
    // get data as integer
    inline int GetAsInteger(int default_return = 0) const {
        try {
            return std::stoi(_Data);
        }
        catch (...) {
            return default_return;
        }
    }

    //
    // set data from float
    inline void SetAsFloat(float value) {
        _Data.assign(std::to_string(value));
    }

    //
    // get data as float
    inline float GetAsFloat(float default_return = 0.f) const {
        try {
            return std::stof(_Data);
        }
        catch (...) {
            return default_return;
        }
    }

public:
    //
    // check if data empty
    bool Empty() const {
        return _Data.empty();
    }

private:
    std::string _Data;
};

//
// util object to restrict copy (and move if not declared move information)
//
class IniNonCopyable {
public:
    IniNonCopyable(const IniNonCopyable&) = delete;
    IniNonCopyable& operator=(const IniNonCopyable&) = delete;
};

//
// container controller base
//
template<class Element_t>
class IniContainer : public IniNonCopyable {
    typedef std::shared_ptr<Element_t>                              ContainerElement_t;
    typedef std::string                                             ContainerKey_t;
    typedef std::unordered_map<ContainerKey_t, ContainerElement_t>  Container_t;

public:
    //
    // default ctor
    IniContainer() {

    }

    //
    // dtor
    virtual ~IniContainer() {

    }

public:
    //
    // access to container element
    // if key does not exist - will be created new one
    Element_t& operator[](const std::string& key) {
        auto it = _Container.find(key);

        if (it != _Container.end()) {
            return *it->second.get();
        }

        auto instance = new Element_t;
        _Container.insert(std::make_pair(key, instance));

        return *instance;
    }

    //
    // check if container empty
    bool operator!() const {
        return Empty();
    }

public:
    //
    // create new element
    // if key already exist will be returned empty element
    virtual Element_t& Create(const std::string& key) {
        if (Has(key)) {
            static Element_t empty;
            return empty;
        }

        auto instance = new Element_t;
        _Container.insert(std::make_pair(key, instance));

        return *instance;
    }

    //
    // delete existed element
    virtual bool Delete(const std::string& key) {
        auto it = _Container.find(key);

        if (it == _Container.end()) {
            return false;
        }

        _Container.erase(key);
        return true;
    }

    //
    // access to container element
    // if key does not exist - will be returned empty element
    virtual Element_t& At(const std::string& key) const {
        auto it = _Container.find(key);

        if (it == _Container.end()) {
            static Element_t empty;
            return empty;
        }

        return *it->second.get();
    }

    //
    // check if element at key exist
    virtual bool Has(const std::string& key) const {
        return _Container.find(key) != _Container.end();
    }

    //
    // container clear
    virtual void Clear() {
        _Container.clear();
    }

    //
    // check if container empty
    virtual bool Empty() const {
        return _Container.empty();
    }

public:
    //
    // return ref to container data
    virtual const Container_t& GetContainer() const {
        return _Container;
    }

protected:
    Container_t         _Container;
};

//
// section handler (IniEntry container)
//
class IniSection : public IniContainer<IniEntry> {
public:
    //
    // default ctor
    IniSection() {

    }

    //
    // dtor
    virtual ~IniSection() {

    }
};

//
// file handler (IniSection container)
//
class IniFile : public IniContainer<IniSection> {
public:
    //
    // default ctor
    // if path not empty then loading will be started automatically
    explicit IniFile(const std::string& path = "") : _Path(path) {
        if (!_Path.empty()) {
            Load();
        }
    }

    //
    // dtor
    virtual ~IniFile() {

    }

public:
    //
    // file parsing
    // if path empty, then will be used ctor initialized path
    bool Load(const std::string& path = "") {
        const std::string file_path(path.empty() ? _Path : path);

        if (file_path.empty()) {
            return false;
        }

        std::ifstream file_stream(file_path);

        if (!file_stream) {
            return false;
        }

        const std::regex comment_regex(R"x(\s*[;#])x");
        const std::regex section_regex(R"x(\s*\[([^\]]+)\])x");
        const std::regex value_regex(R"x(\s*(\S[^ \t=]*)\s*=\s*((\s?\S+)+)\s*$)x");

        std::string file_line;
        std::smatch result;
        std::string section;

        while (std::getline(file_stream, file_line)) {
            if (file_line.empty() || std::regex_match(file_line, result, comment_regex)) {

            }
            else if (std::regex_match(file_line, result, section_regex) && result.size() == 2) {
                section.assign(result[1]);
                Create(section);
            }
            else if (std::regex_match(file_line, result, value_regex) && result.size() == 4) {
                At(section)[result[1]] = result[2].str().c_str();
            }
        }

        return true;
    }

    //
    // file saving
    // if path empty, then will be used ctor initialized path
    bool Save(const std::string& path = "") {
        const std::string file_path(path.empty() ? _Path : path);

        if (file_path.empty()) {
            return false;
        }

        std::ofstream file_stream(file_path);

        if (!file_stream) {
            return false;
        }

        for (auto& section : GetContainer()) {
            file_stream << "[" << section.first << "]" << std::endl;

            for (auto& entry : section.second->GetContainer()) {
                file_stream << entry.first << " = " << (const char*)*entry.second << std::endl;
            }

            file_stream << std::endl;
        }

        return true;
    }

private:
    std::string _Path;
};

The code has declaration / implementation in the same file, just because it's not finished yet. In the future, of course, those will be separated.

Example of usage:

int main() {
    IniFile file("Test.ini");

    if (!file) {
        std::cout << "failed to load" << std::endl;
        return 0;
    }

    auto& general = file["General"];

    if (!general) {
        std::cout << "failed to pick section" << std::endl;
        return 0;
    }

    auto title  = general["Title"].GetAsString("Example");
    auto rate   = general["Rate"].GetAsInteger(100);
    auto scale  = general["Scale"].GetAsFloat(200.f);

    // update and save example

    general["Rate"] = (int)general["Rate"] + 1;

    if (!file.Save()) {
        std::cout << "failed to save" << std::endl;
        return 0;
    }
}

Answer 1

We're missing some required header includes:

#include <fstream>
#include <memory>
#include <regex>
#include <string>
#include <unordered_map>
#include <utility>

---

The code doesn't compile until we give IniNonCopyable an accessible default constructor:

protected:
    IniNonCopyable() = default;

Classes that inherit from IniNonCopyable should inherit it privately.

---

Names that begin with underscore followed by an uppercase letter are reserved for any purpose, which means that they may be pre-defined as macros, leading to very obscure bugs; avoid that naming convention.

---

It seems strange that IniEntry can store float but not double; similarly, char* but not std::string.

Its move and copy constructors and assignment, and destructor, all add no value and should be omitted.

---

Modern C++ style prefers using to typedef:

using ContainerElement_t = std::shared_ptr<Element_t>;
using ContainerKey_t = std::string;
using Container_t = std::unordered_map<ContainerKey_t, ContainerElement_t>;

This puts the type being defined on the left-hand side, consistently with other C++ definitions.

Be careful defining types that end with _t; double-check that these names are not reserved as they are in C.

---

We have made the code much harder to test by not providing load/save methods that work on an already opened stream - we could use those to write and read an in-memory std::strstream object. Keep the filename interface as a slim wrapper.

---

When we load and save, we check the state of the stream immediately after opening, but we should also check at the end, after closing. Note that we'll require an explicit close() call if we want to be sure that we've fully checked the stream state.

Answer 2

Honestly, .ini is an outdated format that is considered deprecated by Microsoft.

Anyways, you don't actually need to write anything - you can use boost::ptree - property tree which can parse (read and write) ini, json, and xml formats. And it is fairly simple to use. The only reason I don't use it in my code is because I learned of it only recently.

At any rate I happen to also use ini format but I found the whole idea of using section+key to access data to be a poor choise. Instead I simply store all data in map<string,string> in format cfg["section/key"] = "value";. It deviates to some degree from classical ini format as one can effectively create subsections (e.g., cfg["section/subsection/key"] = "value";) as well as address values directly without sections. This way ini format becomes as powerful as xml and json - and there are major advantages to usage of such formats over classical ini. Why else do you think .ini is deprecated by Microsoft?

Alright, about your code: I don't see why you need to make the IniSection and IniFile to have virtual destructors and virtual functions. Do you really intend to make derived classes that are gonna override these?

Furthermore, it is better to write IniFile() = default; and virtual ~IniFile() = default; instead of IniFile(){}; and virtual ~IniFile(){};; usually it is unimportant (as well as in the current case) but for trivial classes it can ruin performance on allocation/deallocation operations - as long as they have such class as a member or derive from it.

It is generally troublesome in C++, but initializing file loading from std::string is not a really good idea. It is fine for development code but generally you need support for non-english text. In C++20 you could simply use std::u8string while in current versions consider a path class as input - like one from boost::filesystem or one from C++17 std filesystem library.

Furthermore, your input/output format shouldn't be int/float but rather long long/double (or even long double) as otherwise you might lose data. And this isn't a class where you need performance - rather compatability is of greater value here.

Since compatability > performance here and likely several classes in multi-threaded environment might use the same ini file class instance, you'd better make it thread-safe via std::mutex.