1
\$\begingroup\$

So I needed to represent by objects in terms in ini format and change their values from the ini as well, so I thought why don't implement a property system for this. I want it to be bit fast so I would really like if anyone can remove the std::function if possible but its not necessary. The target application is GUI so small performance should not be a big issue, but still, I think this can be done in a faster way but I can't think of any. no one likes lag. Also, the properties are around 100 or more

#include <functional>
#include <stdexcept>
#include <string>
#include <type_traits>
#include <unordered_map>
#include <variant>

#include <QDebug>

template <typename C> class PropertySet {
public:
    using PropertyValue = std::variant<bool, int, float, std::string>;

    void set(const std::string &prop, PropertyValue v) {
        auto p = m_properties.find(prop);
        if (p == m_properties.end()) {
            throw std::runtime_error("Invalid property key");
        }
        // p.value().setter(v);
        p->second.setter(v);
    }

    void set(const std::string &prop, const std::string &v) {
        auto p = m_properties.find(prop);
        if (p == m_properties.end()) {
            throw std::runtime_error("Invalid property key");
        }
        // p.value().setter(p.value().fromStr(v));
        p->second.setter(p->second.fromStr(v));
    }

    std::unordered_map<std::string, std::string> getAllValues() {
        std::unordered_map<std::string, std::string> r;
        qDebug() << m_properties.size();
        for (const auto &p : m_properties) {
            // r[p.key()] = p.value().toStr();
            r[p.first] = p.second.toStr();
        }
        return r;
    }

private:
    struct Payload {
        std::function<std::string()> toStr;
        std::function<PropertyValue(const std::string &)> fromStr;
        std::function<PropertyValue()> getter;
        std::function<void(PropertyValue)> setter;
    };
    std::unordered_map<std::string, Payload> m_properties;

protected:
    template <typename T> static auto fromStr(const std::string &s) {
        if constexpr (std::is_same<T, int>::value) {
            return std::stoi(s);
        } else if constexpr (std::is_same<T, bool>::value) {
            return s == "true" || s == "1";
        }
        static_assert("Error");
    }

    template <typename T, typename Getter, typename Setter>
    void add(const std::string &name, Getter getter, Setter setter) {
        auto cptr = static_cast<C *>(this);
        Payload p;
        p.toStr = [getter, cptr]() { return std::to_string(std::invoke(getter, cptr)); };
        p.fromStr = [](const std::string &s) { return PropertySet::fromStr<T>(s); };
        p.getter = [getter, cptr]() { return PropertyValue(std::invoke(getter, cptr)); };
        p.setter = [setter, cptr](PropertyValue v) { std::invoke(setter, cptr, std::get<int>(v)); };
        m_properties[name] = p;
    }
};

Also, I'm using Qt, so I was thinking of replacing std::string with QString or QByteArray and std::unordered_map with QHash.

\$\endgroup\$
2
\$\begingroup\$

There are several ways to approach this.

The classic object oriented way is to make Payload be a simple interface class

class IPayload {
    virtual ~IPayload() = default;
    virtual std::string   toStr() = 0;
    virtual PropertyValue fromStr(const std::string &) = 0;
    virtual PropertyValue get() = 0;
    virtual void          set(PropertyValue) = 0;
}; 

And just create for each property its own instance of an implementation of the interface - depending on what kind of property one uses. This can be helpful when you need variety updating mechanisms to work simultaneously. Say, one Payload that stores the field only in memory, one that stores into both in memory and in a file, and one that forwards it to GUI - or a combination of these. This can be also helpful when you require specialized treatment for some properties - like time input property - that upon receiving std::string input it processes it according to some time conversion, i.e., read double and then converts it to seconds depending on the time unit specification, e.g., "5min" converts to 300 (seconds). The only issue is that you ought to instantiate a treatment for each property - which can be a hassle and make the class unsuitable for some trivial basic usages.

I don't see any circumstances when the std::function approach would better - unless one has to runtime change setters or something - which is kinda odd honestly.

Also you can try going in the opposite direction and detach the storage class from the "customized actions". Say, make a simple multipurpose ini file parser wrapping std::unordered_map<std::string,std::string> - or use an existing one like boost::ptree that can read and write xml and json in addition. While in the GUI simply call the right functions when user sets the fields. Whether it is viable / not viable / preferred option depends a lot on your development platform for the GUI as well as other nuances.

You can in fact combine the two methods. Use the boost::ptree for storing / parsing and for internal usage whilst class PropertySet make responsible for the GUI handling and just instantiate one from each other upon loading / saving / updating.

| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ I'm finding std::function release build faster, but debug bit slower, maybe debug std::function does some unnecessary checking \$\endgroup\$ – bluedragon Mar 14 at 9:32
  • 1
    \$\begingroup\$ @bluedragon std::function performs a virtual call itself. Only that each of these std::functions has its own vtable - so it is very surpring that std::function performs faster. Not only that, but it makes a call that calls the actual function. Consider asking for another review and ask to compare these two implementations. I can't tell much beyond as I don't see the code. \$\endgroup\$ – ALX23z Mar 14 at 9:55
  • \$\begingroup\$ my guess is std::function does small function optimization and doesn't do dynamic allocation while my every payload was dynamically allocated here is my implementaion pastebin.com/vEjvPF3B \$\endgroup\$ – bluedragon Mar 14 at 11:04
  • 1
    \$\begingroup\$ @bluedragon I am confused about the implementation you wrote. Why std::shared_ptr<IPayLoad> inside a two layered unordered map? If you shared all instances inside the map then I could understand but no - you create a single instance for each of the shared pointer. ... Second - probably more important part. Each virtual function get/set calls some getter/setter - this is a double function call - not ideal. Unless your getter/setter is an optimazable lambda you are unlikely to see any performance improvement from this. \$\endgroup\$ – ALX23z Mar 17 at 1:58
  • 1
    \$\begingroup\$ @bluedragon I have some doubts about the double unordered_map. I am not 100% sure but I believe that it would've worker better if it was a single undordered_map and you accessed value at (group_name,name) via a single string group_name/name. \$\endgroup\$ – ALX23z Mar 17 at 2:06

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.