2

The below class is meant to be a top-layer class which brings all the benefits of nlohman::json but offers additional functions.

#include <nlohmann/json.hpp>

class Other { /* ... */ };

class AbstractData : public nlohmann::json
{
public:
    AbstractData (const nlohmann::json& json) : nlohmann::json(json) { }

    Other createOther(const char* key) { /* create Other class using key */ }
    std::string toString() { /* convert to string */ }
    /* etc. */
};


But I ran into issues when using operator[]. By default we have

AbstractData a;
auto& val = a["some_key"];  // val is nlohman::json::value_type&

and thus val loses all the extra functions.

When we provide a class function operator[]

const AbstractData& AbstractData::operator[](const char* key) const
{
    return nlohmann::json::operator[](key);
}

then

AbstractData a;
auto& val = a["some_key"];  // val is AbstractData&

works as expected. But in order to achieve this, the copy constructor AbstractData (const nlohmann::json& json) is called (which is very inefficient for large objects). And this defeats the purpose of returning a reference in the first place.

I've seen questions like this one Add a method to existing C++ class in other file but they didn't offer help with my specific problem.

Any advice?

Guillaume Racicot
  • 39,621
  • 9
  • 77
  • 141
Phil-ZXX
  • 2,359
  • 2
  • 28
  • 40
  • 1
    `https://github.com/nlohmann/json#design-goals` do not include extensibility. Don't use inheritance for extensions! – xtofl Aug 06 '19 at 14:07

1 Answers1

5

I would drop inheritance and wrap de data completely.

Why? Because the moment you need a second AbstractData, you will have to hold and potentially copy the json value. If you wrap the json data instead of using inheritance, then you can act as a view over json data.

class AbstractData {
    // view over json
    nlohmann::json const* _json_ptr;

    auto json() -> nlohmann::json const& {
        return *_json_ptr;
    }

public:
    AbstractData(nlohmann::json const& json) : nlohmann::json(&json) {}

    Other createOther(const char* key) {
        /* create Other class using key */
    }

    std::string toString() {}

    auto operator[](const char* key) const -> AbstractData {
        return AbstractData{&(json()[key])};
    }
};

As you can see you can safely return by value, since your class only holds a pointer to your value and is cheap to copy.


If you also want your class to be the owner, you can store the json as a const shared pointer:

class AbstractData {
    using root_t = std::shared_ptr<nlohmann::json const>;
    // owner of the json root.
    root_t _root;

    // view over json
    nlohmann::json const* _json_ptr;

    auto json() -> nlohmann::json const& {
        return *_json_ptr;
    }

    AbstractData(nlohmann::json const& json, root_t root) :
        _root(root), _json_ptr(&json) {}

public:
    struct new_root_t {} static constexpr new_root{};

    AbstractData(new_root_t, nlohmann::json json) :
        _root{std::make_shared<nlohmann::json const>(std::move(json))}, _json_ptr{_root.get()} {}

    auto operator[](const char* key) const -> AbstractData {
        // always pass down the root, so someone will own it
        return AbstractData{json()[key], _root};
    }
};

Live example


As a side note, you had undefined behaviour:

// return by reference?
const AbstractData& AbstractData::operator[](const char* key) const {
    // construct a new, local value
    // the local value is destroyed then returned
    return nlohmann::json::operator[](key);
}

I strongly suggest to return by value here.

Guillaume Racicot
  • 39,621
  • 9
  • 77
  • 141
  • I like this idea, but where does `nlohmann::json const* _json_ptr` actually live? Who owns it? If I call `AbstractData(nlohmann::json{})` it will store the address, but then destroy the object (similar to what you've pointed out in my UB case). As for `operator[]`, this should ideally ALWAYS return a reference because that's the efficient way (no copies are being constructed, etc.) – Phil-ZXX Aug 06 '19 at 13:16
  • 1
    @Phil-ZXX oh in fact this is really bad, you have double free and stuff, this is not how you use shared pointers. – Guillaume Racicot Aug 06 '19 at 13:55
  • @Phil-ZXX I added the live example. When creating a new tree, it is now explicit and I fixed the double free with the shared pointer. – Guillaume Racicot Aug 06 '19 at 14:03
  • Ah, I see what you did. And good idea to make the ptr constructor private. – Phil-ZXX Aug 06 '19 at 14:28