Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Plugin system #599

Closed
an-kumar opened this issue May 31, 2017 · 11 comments
Closed

Plugin system #599

an-kumar opened this issue May 31, 2017 · 11 comments

Comments

@an-kumar
Copy link

an-kumar commented May 31, 2017

Many header-only libraries (for example, cimg) include 'plugin' systems, where people can add functionality to the class. It looks something like this:

class some_class
{
// normal code
#ifdef PLUGIN
#include PLUGIN
#endif
// more code
};

Could we add something like this to this json library? I have some added functions that force me to maintain a version of json.hpp checked into my repo, but this makes it awkward to merge in updates to this repo.

@nlohmann
Copy link
Owner

Could you provide a concrete example of such an extension?

@an-kumar
Copy link
Author

an-kumar commented May 31, 2017

Here are some examples. They're pretty minimal one but i think you'd get the idea of what you'd be able to do.

First, here's an example of overloaded the value() function for a specific class, allowing extra parsing in order to find the right way to create the class. (the following is pseudo code as I don't actually have this in my json file)

// template specialization for reading blob class from json
template <>
BlobClass value(const typename object_t::key_type& key, BlobClass default_value) const
{
if (find(key+"_file")) return BlobClassFromFile(this->operator[](key+"_file"));
else if (find(key+"_raw")) return BlobClassFromBase64EncodedString(this->operator[](key+"_raw"))
else
return BlobClassFromString(this->operator[](key));
}

There are other examples related to interfacing the json class with external classes.

There are also, of course, some simple functions to add that could otherwise be put into a pull request, but maybe they aren't so necessary to get put into the main repo, so this would allow people to add these functions locally. Here, for example, are two that I have:

// similar to at(), but nonconst, and allows a default argument which is placed in the object if the key is not found
reference at(const typename object_t::key_type& key, const_reference def)
    {
        // at only works for objects
        if (is_object())
        {
            JSON_TRY
            {
                return m_value.object->at(key);
            }
            JSON_CATCH (std::out_of_range&)
            {
                JSON_TRY
                {
                  this->operator[](key) = def;
                  return m_value.object->at(key);
                }
                JSON_CATCH (std::out_of_range&)
                {
                  // create better exception explanation
                  JSON_THROW(std::out_of_range("key '" + key + "' not found"));
                }
            }
        }
        else
        {
            JSON_THROW(std::domain_error("cannot use at() with " + type_name()));
        }
    }

// simple contains function
bool contains(const typename object_t::key_type& key) const
{
return find(key) != end();
}

@TurpentineDistillery
Copy link

I like this idea.
In my case I'm using custom overloads of at and operator[] for string-literals that avoid instantiating a std::string on every access. With a plugin approach it would be trivial to inject the code without having to do a merge.

@nlohmann
Copy link
Owner

nlohmann commented Jun 5, 2017

So you would literally add the lines

#ifdef PLUGIN
#include PLUGIN
#endif

to the beginning of the basic_json class?

@TurpentineDistillery
Copy link

I'm thinking it would look like

#ifdef NLOHMANN_JSON_INJECT
#include NLOHMANN_JSON_INJECT
#endif

probably at the end of basic_json rather than the beginning.

@jaredgrubb
Copy link
Contributor

Why is this better than subclassing? Using preprocessor tricks can lead to bugs in ODR (eg, if one file #define's the macro, but another file does not; or if you link two libraries together that have differening things in them).

@an-kumar
Copy link
Author

an-kumar commented Jun 6, 2017

Yes, it would look just like
#ifdef PLUGIN #include PLUGIN #endif

It would probably go at the end of basic_json. You can see examples of a plugin system in Eigen and CImg, both very mature header-only c++ libraries.

@jaredgrubb : this is better than subclassing because of functions returning references. The json class returns references in the form json& -- if you make a class JsonSubclass : public nlohmann::json, there is no way to make the nlohmann::json functiosn return a JsonSubclass& rather than a json&.

@nlohmann
Copy link
Owner

nlohmann commented Jun 7, 2017

Maybe related: #456

@nlohmann
Copy link
Owner

@an-kumar I'm still not really convinced that we need to rely on a preprocessor hack to solve a C++ problem that should have simpler solutions. Are you sure that the reference example you gave is impossible without code injection?

@nlohmann
Copy link
Owner

Another question I have for this: There a several classes inside the basic_json class (lexer, parser, iterators, etc.) - how would this approach help here?

@an-kumar
Copy link
Author

I don't know of another way to do the reference example. Would love to hear a different solution there.

As for the several classes inside basic_json -- yeah, this is a bit of an annoying issue. The way Eigen does this is they have a number of plugin definitions (e.g MATRIXBASE_PLUGIN, DENSEBASE_PLUGIN, etc) for the different classes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants