'data' class doesn't allow assignment from fundamental types?

[nlyan]

I'm trying to update some code from b755c77 to HEAD and I see that the 'value' class has been renamed to 'data', but it seems you cannot simply assign integer values or string literals to this object now.

What's the recommended usage here?

[d-frey]

Basically, I split value into data, which is independent of the traits, and custom_value, which is a view on the stored data defined by the traits.

As I am currently on mobile, I‘ll write more once I am back home. But I am also refactoring this area once again, so you might want to wait a few days for the code to settle.

[nlyan]

Cheers. I look forward to hearing more about the rationale

[d-frey]

OK, I'm back home :)

The key observation that caused the split is, that you might have different traits classes but you want "compatible" instances, i.e., you want to be able to move a value with one traits class to another value with a different traits class. This should be possible since the class that holds the actual data (now appropriately called data) does not itself depend on the traits class used. The traits class is just used to form a "view" or an "interface" onto the data itself.

This also helps to keep the whole events API independent of the traits, another benefit is that specializations of the traits class become easier in some cases as they only operate on the data.

That said, this is all still an experiment and I was just discussing it with Colin whether we want to keep it or roll back to a "simple" basic_value< Traits >-approach. But after some discussion I think I can refactor the current split and make it both more usable as well as apply some DRY-principle to the code itself. This is quite a bit of work and I am in the middle of said refactoring, I hope to finish within the next few days.

For now, if you really want to play with the current code, the old basic_value< Traits > is now called basic_custom_value< Traits > and there is an alias for the default traits traits, called custom_value (which replaces the old value). That might or might not work for you :)

[d-frey]

Good news! I found some time to finish the refactoring and finally renamed custom_value back to value. Plus the new code should be way more complete than the intermediate version from before.

Can you test if you can upgrade from the pre-split code to HEAD now? Thanks for your patience :)

[nlyan]

Not too bad. I had to change my deserialization code to take json::data& rather than json::value&. The only other pattern that i had to change was this

json::value foo;
auto& bar = foo["bar"];

to

auto bar = foo["bar"];

I'm not sure how I feel about the ownership semantics being obscured here

[d-frey]

I am still not sure whether the split was/is worth it. As you can see, some things become a lot more messy but I guess that can't be avoided if I want to split the data from the covenience interface.

On the plus side, the whole library is now free from the traits and a lot less templating is going on. value.hpp is the only header including traits.hpp and no other header includes value.hpp - it is purely optional. OTOH, almost everyone is going to use it as the data class is quite inconvenient to use.

What is your feeling about this? Do you like the split or do you think it is over-engineered?

[nlyan]

Not over-engineered, I think have a parsed type-erased JSON data type and a type-safe view in to that is a good idea in principle

That said, I'm currently not directly using any of the traits capability because I'm using the library in combination with Boost Fusion to allow easy and typesafe deserialization of BF adapted structs. Here's an example function from one of my overload set

template <typename T>
inline void
from_json (std::vector<T>& dest, tao::json::data& src,
           char const* const field_name = "") {
    if (SCS_UNLIKELY (!src.is_array ())) {
        throw std::runtime_error (
            fmt::format ("Expected field '{}' to be an array, but found a "
                         "value of '{}' type",
                         field_name,
                         tao::json::to_string (src.type ())));
    }

    auto& json_array = src.get_array ();
    for (auto& element : json_array) {
        dest.emplace_back ();
        from_json (dest.back (), element);
    }
}

My top level function uses the boost::fusion::traits::is_sequence trait to iterate over struct members and perform extraction recursively for vectors, boost::optionals, integral types, and bools, with members looked up using the B.F. struct_member_name extension

Integral types are handled via a single overload using enable_if and Boost numeric_cast for bounds checking.

Defining a JSON struct then boils down to this

SCS_DEFINE_JSON (UserCredentials, (std::string, email) (std::string, password));

UserCredentials creds;
from_json (creds, R"JSON({"email": "", "password": ""})JSON");

[d-frey]

Well, I just discussed the split with Colin and we finally agreed to roll it back. I'll merge data and basic_value again, as our data is a hierarchical value where each node may contain other nodes - that makes wrapping the interface a real pain in the backside.

It was an important and interesting experiment, though, and I'll keep some of the smaller improvements. As this is quite some work, I'll need some time.

Again, I'm sorry for the inconvenience and I'll update (and finally close) this issue once the rollback is done. I am hopeful that no further changes of this magnitude will happen in the future, the library will now stabilize towards a 1.0.0 release. (which still requires a lot of work, especially documentation :)

[d-frey]

OK, I've rolled back the split - you can now use basic_value< Traits >, or just value, like before.

[nlyan]

The rollback worked perfectly. Thanks for working so hard on both this library and PEGTL, they're both awesome.

[d-frey]

Thanks, your feedback is appreciated :)