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

Convenient way to make 'basic_json' accept 'QString' as an key type as well? #1640

Closed
FlKo opened this issue Jun 16, 2019 · 6 comments
Closed
Labels
kind: question state: stale the issue has not been updated in a while and will be closed automatically soon unless it is updated

Comments

@FlKo
Copy link

FlKo commented Jun 16, 2019

I want to use Qt's QString type with the excellent nlohmann::json.
This works very well by providing the functions to_json and from_json as described in the readme:

nljson.h

#include <QString>
#include <QByteArray>
#include <vector>
#include "nlohmann/fifo_map.hpp"
#include "nlohmann/json.hpp"

// This is a workaround solution for the "nlohmann::json" library to preserve
// the insertion order of JSON objects instead of sorting them by name.
// It is based on a custom map implementation by Niels Lohmann itself
// (nlohmann::fifo_map). The actual solution (by GitHub user "Daniel599")
// can be found here: https://github.com/nlohmann/json/issues/485#issuecomment-333652309
template<class K, class V, class dummyCompare, class A>
using nljson_fifo_map = nlohmann::fifo_map<K, V, nlohmann::fifo_map_compare<K>, A>;
using nljson = nlohmann::basic_json<nljson_fifo_map>;

QT_BEGIN_NAMESPACE
void to_json(nljson &j, const QString &qstr);
void from_json(const nljson j, QString &qstr);

void to_json(nljson &j, const QByteArray &qba);
void from_json(nljson &j, QByteArray &qba);
QT_END_NAMESPACE

nljson.cpp

#include "qnljson.h"

#include <QString>
#include <QByteArray>
#include <string>
#include "nlohmann/json.hpp"

QT_BEGIN_NAMESPACE

void to_json(nljson &j, const QString &qstr)
{
    j = nljson { qstr.toStdString() };
}

void from_json(const nljson j, QString &qstr)
{
    if (j.type() == qnljson::value_t::string)
        qstr = QString::fromStdString(j.get<std::string>());
    else
        qstr = QString::fromStdString(j.dump());
}

void to_json(nljson &j, const QByteArray &qba)
{
    j = qnljson { qba.toStdString() };
}

void from_json(nljson &j, QByteArray &qba)
{
    qba = QByteArray::fromStdString(j.get<std::string>());
}

QT_END_NAMESPACE

However, I would find it very beneficial if it were possible to use QString as a key data type as well:

myfile.cpp

// {...}
QString foo = "abc";
nljson bar { "a", foo };
QString baz = "a";

qDebug() << bar.at(baz).get<QString>(); // Error: no viable conversion from 'QString' to 'int'

qDebug() << bar.at(baz.toStdString()).get<QString>(); // This works

Is there a convenient way to make nlohmann::json accept QString as a key type without this explicit conversion (baz.toStdString())?

@nickaein
Copy link
Contributor

I haven't tested this but how about overriding the StringType from std::string to QString?

using my_json_type = nlohmann::basic_json<std::map, std::vector, QString>;

my_json_type json;

QString key = "hello";
json[key] = "world";

I am not sure if QString provides all APIs required by the library though.

@FlKo
Copy link
Author

FlKo commented Jun 16, 2019

Unfortunately when I tried that:

// {...}
template<class K, class V, class dummyCompare, class A>
using nljson_fifo_map = nlohmann::fifo_map<K, V, nlohmann::fifo_map_compare<K>, A>;
//using nljson = nlohmann::basic_json<nljson_fifo_map>;
using nljson = nlohmann::basic_json<nljson_fifo_map, std::vector, QString>;
// {...}

the result was this compilation error (MSVC 2015 64-Bit):

C2338: The C++ Standard doesn't provide a hash for this type.

so at least the hash-thing is missing.

Are there any chances to provide this hash?

@FlKo
Copy link
Author

FlKo commented Jun 16, 2019

I am also afraid that if I use QString directly as the common string type, there may be errors (QString is UTF-16 internally).

The Readme states about string encodings:

Note the library only supports UTF-8. When you store strings with different encodings in the library, calling dump() may throw an exception unless json::error_handler_t::replace or json::error_handler_t::ignore are used as error handlers.

@nickaein
Copy link
Contributor

nickaein commented Jun 16, 2019

Are there any chances to provide this hash?

This solution might work.

I gave QString a try and there are some missing APIs (e.g. like c_str()). Considering these, the issue you pointed out about QString use of UTF-16, and the previous discussion at #274 I'm afraid that there is probably not a straightforward solution use QString as std::string at least for now. I can imagine two options to proceed:

  1. Implement a class that provides the minimal API that this library needs for a StringType and also support implicit conversion from QString. Use this class in instantiation of basic_json template for StringType instead of QString.

  2. Leave the default nlohmann::json as it is (with std::string as StringType) and do all conversions from QString to std::string manually.

Option 2 is a faster and less hassle solution if you can minimize the instances where you have to modify/query the JSON object. But if there are many places that you have to do this conversion, Option 1 might worth trying as it encapsulates all those conversions in one place and would be easier to maintain.

@nlohmann
Copy link
Owner

I agree with #1640 (comment). The library only has superficial support for other string types, especially if they do not have the API of std::basic_string. The approaches described in #1640 (comment) are the only options as far as I see.

@stale
Copy link

stale bot commented Jul 24, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the state: stale the issue has not been updated in a while and will be closed automatically soon unless it is updated label Jul 24, 2019
@stale stale bot closed this as completed Jul 31, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind: question state: stale the issue has not been updated in a while and will be closed automatically soon unless it is updated
Projects
None yet
Development

No branches or pull requests

3 participants