6
\$\begingroup\$

When sections are ignored and no writing capability is needed, an INI reader becomes quite simple to implement. Here is my attempt at reading a sectionless INI file without any non-std dependencies.

The general idea is to store all key-value pairs in a <string, string> map, and use getters to convert the value string to desired type on demand.

#include <fstream>
#include <iostream>
#include <map>
#include <string>

class SectionlessINIReader {

private:
    std::map<std::string, std::string> dict;

public:

    // https://stackoverflow.com/a/1798170/3516684
    static std::string trim(const std::string& str,
                 const std::string& whitespace = " \t")
    {
        const auto strBegin = str.find_first_not_of(whitespace);
        if (strBegin == std::string::npos)
            return ""; // no content

        const auto strEnd = str.find_last_not_of(whitespace);
        const auto strRange = strEnd - strBegin + 1;

        return str.substr(strBegin, strRange);
    }

    SectionlessINIReader(const std::string& filename) {

        std::ifstream file(filename);
        std::string line;
        int idxEq;
        while (std::getline(file, line))
        {

            if ((line.find_first_not_of(";#")==0) &&
                ((idxEq = line.find("=")) != std::string::npos)) {

                    std::string key = trim(line.substr(0,idxEq));
                    std::string value = trim(line.substr(idxEq+1));

                    dict[key] = value;
                }
        }
        file.close();
    }

    std::string get(std::string key) {
        return dict[key];
    }

    bool getLong(std::string key, long* p_value) {

        std::string value = dict[key];

        if (value=="") {
            return false;

        } else {
            *p_value = std::stol(dict[key]);
            return true;
        }
    }

    // bool getDouble(...)

};

The intended usage is:

SectionlessINIReader ini("config.ini");
long age;
if (ini.getLong("age", &age))
    std::cout << "Name: " << ini.get("name") << "; age: " << age << std::endl;
\$\endgroup\$
3
\$\begingroup\$

Include (only) what you use

This class uses nothing from <iostream>, so it's a waste of the compiler's time to include it.

Control the public interface

Is trim() really a facility that should be a part of SectionlessINIReader? I don't think it belongs in the public interface, and more likely belongs in a private or protected section.

Make it easier to use

The getLong() member function has an awkward interface, requiring the user to pass an lvalue (worse, a pointer). Consider returning a std::optional<long> instead, to convey the result in a single value.

Pass strings by const reference

There's no need to make a copy of the key argument to the accessors - that should be declared as const std::string& key instead.

Accessors should be const

get() and getLong() both modify dict (they create a new empty mapping for any requested key that doesn't yet exist). We don't want to do that, as it's wasteful. We would have spotted this bug if we'd declared those members const (so that they could be used by code with only a const reference to the reader - that will be important if we later add writing functions).

We could write:

std::optional<std::string> get(std::string key) const
{
    auto const it = dict.find(key);
    if (it == dict.end())
        return {};

    return it->second;
}

And getLong() could use it:

std::optional<long> getLong(std::string key) const
{
    auto const value = get(key);
    if (!value.has_value())
        return {};

    return std::stol(value.value());
}

Open the code to extension

At present, the only way to add more types that can be read is to edit this class or to subclass it. And it's not designed for subclassing (it has a non-virtual destructor), so it's closed to end-users.

One way to make it open for extension is to separate the string-to-value conversion from the string lookup, and allow users to specialize that part; our code could then use the appropriate specialization, or fail to compile if no conversion has been defined.

This would be the starting position:

namespace convert {

    // This function must be specialized for any extension types to
    // convert from string to value.
    template<typename T>
    std::optional<T> fromString(const std::string&);
}

It's used in our get<>() function quite simply:

template<typename T>
std::optional<T> get(const std::string& key) const
{
    auto const value = get(key);
    if (!value.has_value())
        return {};

    return convert::fromString<T>(value.value());
}

We could provide some common conversions:

template<>
std::optional<std::string> convert::fromString(const std::string& s)
{ return s; }

template<>
std::optional<long> convert::fromString(const std::string& s)
{ return std::stol(s); }

template<>
std::optional<double> convert::fromString(const std::string& s)
{ return std::stod(s); }

Users can add their own conversions in similar manner.

Variation

Instead of creating a free function to specialize, we could make fromString() be a private static member function of SectionlessINIReader. The choice may well depend on whether you might want to use fromString() from anywhere else.

Think about exceptions

std::stol() will throw an exception if the value can't be parsed as a long. Do we want to pass this straight to our callers, or do we want to treat malformed values the same as missing values, and simply return an empty std::optional? (There's no right or wrong answer here; I just want to be sure that we've made a conscious choice.)

Avoid narrowing conversions

    int idxEq;
    ...
        if (((idxEq = line.find("=")) != std::string::npos)) {

Here, we assign the result of find() (a size type) to idxEq (a signed integer) and then compare idxEq to std::string::npos (a size type again). That's a potentially narrowing conversion which could change the sign, and a comparison between types of different signedness. I know we don't expect keys to be long enough to overflow int, but it would be nice to silence the compiler warning here, and it's very easy to make it consistent:

    std::string::size_type idxEq;

Check the result of filesystem operations

In the constructor, we call file.close(), but never check the result. If we want to be sure that we read the complete file, we need to ensure that close() has succeeded.

If we really don't care about whether we missed some data, then we could simply omit the close() and let the file's destructor do that for us - but I don't advise ignoring a failed close.

Inefficient use of search

There's no need to search all the way through a string just to see what its first character is:

if (line.find_first_not_of(";#") == 0)

It's clearer to examine the first character directly (after checking that the line has at least one character, of course).

Check for duplicate keys

When reading, we simply overwrite any key that appears more than once. Consider emitting a warning, or throwing an exception.

Remove dead code

There's a remnant of some old code still hanging around, which should be deleted:

// bool getDouble(...)

Modified code

#include <fstream>
#include <map>
#include <optional>
#include <stdexcept>
#include <string>

class SectionlessINIReader
{

private:
    std::map<std::string, std::string> dict = {};

public:
    SectionlessINIReader(const std::string& filename)
    {
        auto file = std::ifstream {filename};
        readStream(file);
    }

    SectionlessINIReader(std::istream& in)
    {
        readStream(in);
    }

    std::optional<std::string> get(const std::string& key) const
    {
        auto const it = dict.find(key);
        if (it == dict.end())
            return {};

        return it->second;
    }

    template<typename T>
    std::optional<T> get(const std::string& key) const
    {
        auto const value = get(key);
        if (!value.has_value())
            return {};

        return fromString<T>(value.value());
    }

private:

    // https://stackoverflow.com/a/1798170/3516684
    static std::string trim(const std::string& str,
                            const std::string& whitespace = " \t")
    {
        const auto strBegin = str.find_first_not_of(whitespace);
        if (strBegin == std::string::npos)
            return {}; // no content

        const auto strEnd = str.find_last_not_of(whitespace);
        const auto strRange = strEnd - strBegin + 1;

        return str.substr(strBegin, strRange);
    }

    template<typename T>
    static std::optional<T> fromString(const std::string&);

private:

    void readStream(std::istream& in)
    {
        std::string line;
        while (std::getline(in, line)) {
            if (line.empty() or line[0] == ';' or line[0] == '#') {
                continue;
            }
            auto const idxEq = line.find('=');
            if (idxEq == line.npos) {
                // consider reporting an error here
                continue;
            }

            auto const key = trim(line.substr(0,idxEq));
            auto const value = trim(line.substr(idxEq+1));
            if (!dict.emplace(key, value).second) {
                throw std::runtime_error("Duplicate key in INI file");
            }
        }
        if (!in.eof()) {
            throw std::runtime_error("Failed to read INI file");
        }
    }
};

// String converters
template<>
std::optional<std::string> SectionlessINIReader::fromString(const std::string& s)
{ return s; }

template<>
std::optional<long> SectionlessINIReader::fromString(const std::string& s)
{ return std::stol(s); }

template<>
std::optional<double> SectionlessINIReader::fromString(const std::string& s)
{ return std::stod(s); }

// we can add more specializations here, and user may also add
// specializations.
#include <iostream>
#include <sstream>
int main()
{
    std::istringstream contents("age = 30\n"
                                "favourite_drink = tea\n"
                                "name = Bob\n");

    const SectionlessINIReader ini(contents);
    auto age = ini.get<long>("age");
    if (age.has_value())
        std::cout << "Name: " << ini.get<std::string>("name").value_or("")
                  << "; age: " << age.value()
                  << std::endl;
}
\$\endgroup\$
  • \$\begingroup\$ why if (!in.eof()) and not if (!in.good())? \$\endgroup\$ – Sparkler Nov 8 '18 at 14:59
  • \$\begingroup\$ @Sparkler: Because good() is always false at the end of file (where we believe we should be). Look at the truth table at the foot of the std::ios_base::iostate documentation if that helps. Or just copy this code and try it for yourself! \$\endgroup\$ – Toby Speight Nov 8 '18 at 15:09
  • \$\begingroup\$ thanks for the link! in that case, why not have (!in.eof()) || (in.fail())? \$\endgroup\$ – Sparkler Nov 8 '18 at 15:36
  • \$\begingroup\$ That might be a better choice, but I tried that and failbit was set at the end of reading from the stringstream. So I just stuck with something that seems to work (actually, I'm not sure how to effectively test the error cases; might need to invent a mockable stream with controlled failure points). \$\endgroup\$ – Toby Speight Nov 8 '18 at 15:49
2
\$\begingroup\$

Code Review

There is no need to close the file manually, it will be closed as part of the std::ifstream's destructor.

Checking if line is comment could be simplified into checking the first character and oring the result.

There is a character overload of std::string::find, which is at least more precise in conveying intent.

In getLong(), dict[key] might be an empty string, and it is possible to find such value in ini file if the equal sign is the last character of the line. As such, it is uncertain what the function should do, but I guess current implementation does the least surprising thing.

int sometimes might be a bit too small, std::size_t is the current default for index type. There are gossips about std::index coming, but it doesn't seem to be near future.

If the code accepts a raw pointer, one should expect to get a nullptr until otherwise documented. My first impression would be that I could pass nullptr to check if there is a long with that key, but in reality the world will come to its end, in the best scenario.

Interface

I'd prefer to accept std::istream& input_stream in the constructor, rather than a file name, but it is purely subjective opinion with current context.

Well, I'm uncertain how this class should be used. In my opinion, it should lean strongly on open to extension, but close to modification principle, as there might be multiple types serialized into ini file. I see multiple reasonable roads:

  1. Extension by inheritance

    For this, dict must be protected member of the class. I don't see much benefits of this approach, other than being more intuitive to beginner C++ programmers only accustomed to raw object oriented programming. This might open pandora's box, and lead to things like diamonds of death, object slicing, wrong reinterpret_casts and whatnot. But correct code will still be hiding inside :)

  2. Making whole thing a function returning dict

    This approach is for those who like doing things themselves, but for most this will be a downgrade.

  3. Template metaprogramming, which will scare away most of the maintainers

    Until chariot of Helios, concepts, arrive, we have to fight the darkness using SFINAE.

    template <typename T, typename = std::enable_if_t<std::is_constructible_v<T, const std::string&>, void>
    std::optional<T> ini_cast(const std::string& key) {
        auto key_location = dict.find(key);
        if (key_location == dict.end()) {
            return {};
        }
    
        return T(key_location->second);
    }
    
    template <typename Transformator> //free? SFINAE on decltype :)
    decltype(std::declval<Transformator>()(std::declval<const std::string&>())) transform_value(const std::string& key, Transformator&& transformator) {
        auto key_location = dict.find(key);
        if (key_location == dict.end()) {
            //throw?
        }
    
        return transformator(key_location->second);            
    }
    

    The code above is just a sketch, it might not work at all, but hopefully illustrates the rigorous work needed to make it work the way people expect.

As noted by @TobySpeight, using ADL and calling free function might be a good option too. We'll lose invoke type deduction, but hopefully it will just return T. Read Toby’s answer, the part about opening for extension.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.