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

Add something like Json::StaticString #1402

Closed
ibc opened this issue Dec 23, 2018 · 14 comments
Closed

Add something like Json::StaticString #1402

ibc opened this issue Dec 23, 2018 · 14 comments

Comments

@ibc
Copy link

ibc commented Dec 23, 2018

  • Describe the feature in as much detail as possible.

In JsonCpp I can create a "static" JSON string to be used as a Object key with fast access.

  • Include sample usage where appropriate.
void doStuff(Json::Value& data)
{
  static const Json::StaticString JsonStringFoo{ "foo" };

  if (!data[JsonStringFoo].isString())
    // Fail here somehow.

  std::string foo = data[JsonStringFoo].asString();
}

The thing here is that data[JsonStringFoo] is supposed to be much faster than using data["foo"] all the time.

AFAIS nlohmann/json does not provide something like that, although it may provide a different approach to get the same result.

@nlohmann
Copy link
Owner

I do not see why such a string would be faster. Can you elaborate?

@ibc
Copy link
Author

ibc commented Dec 23, 2018

It seems to be useful when adding fields with same name to an JSON object:

Value constructor and objectValue member assignement takes advantage of the StaticString and avoid the cost of string duplication when storing the string or the member name.

http://open-source-parsers.github.io/jsoncpp-docs/doxygen/class_json_1_1_static_string.html#details

I assume the optimization just happens here:

static const Json::StaticString JsonStringFoo{ "foo" };

Json::Value data1;
Json::Value data2;

data1[JsonStringFoo] = 111;
data2[JsonStringFoo] = 222; // <-- here the "foo" string is not duplicated internally.

@ibc
Copy link
Author

ibc commented Dec 23, 2018

(and also when storing the same string as value).

@nlohmann
Copy link
Owner

When you pass a string to operator[] it is not copied, but passed as const reference. Internally, we use a std::map which, in case no value was stored so far for this key, must create an entry. In this case, also the static string would be copied into the map's tree.

@ibc
Copy link
Author

ibc commented Dec 23, 2018

Understood. May I ask which would be the proper way to check whether a key exists, check its type is the desired one and read the corresponding type value into a C++ variable (without looking for the given key 3 times into the internal map?

@nlohmann
Copy link
Owner

You can use find, just like for a map.

@ibc
Copy link
Author

ibc commented Dec 23, 2018

Do you mean using the Iterator returned by find() to later check the value type and then reading its native value?

@nlohmann
Copy link
Owner

I’m not sure what you mean. Can you provide an example?

@ibc
Copy link
Author

ibc commented Dec 23, 2018

Sure. In libsdptransform (in which I use nlohmann/json) I encourage users to be careful when reading JSON parsed values:

auto h264Fmtp = sdptransform::parseParams(video.at("fmtp")[0].at("config"));
std::string profileLevelId;

if (
  h264Fmtp.find("profile-level-id") != h264Fmtp.end() &&
  h264Fmtp["profile-level-id"].is_string()
)
{
  profileLevelId = h264Fmtp.at("profile-level-id");
}

In this usage example, I'm forcing json library to inspect a key named "profile-level-id" 3 times (in find(), in [] and in at()).

@nlohmann
Copy link
Owner

Once you make sure the iterator is not end(), you can dereference it to a reference where you can call all member functions.

@ibc
Copy link
Author

ibc commented Dec 23, 2018

I wonder if I could do something like:

auto h264Fmtp = sdptransform::parseParams(video.at("fmtp")[0].at("config"));
std::string profileLevelId;

// Iterator
auto it = h264Fmtp.find("profile-level-id");

// Pseudo code here:
if (it != h264Fmtp.end() && it->second.is_string())
  profileLevelId = it->second.get<std::string>();

@ibc
Copy link
Author

ibc commented Dec 23, 2018

oh, so it would be:

auto h264Fmtp = sdptransform::parseParams(video.at("fmtp")[0].at("config"));
std::string profileLevelId;

// Iterator
auto it = h264Fmtp.find("profile-level-id");

// Pseudo code here:
if (it != h264Fmtp.end() && it->is_string())
  profileLevelId = it->get<std::string>();

Am I right?

@ibc
Copy link
Author

ibc commented Dec 23, 2018

...or just:

profileLevelId = *it;

@ibc
Copy link
Author

ibc commented Dec 23, 2018

And this is properly documented in the API, so let's close this issue. Thanks :)

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

No branches or pull requests

2 participants