Json: Support startIndex for from_cbor/from_msgpack

Created on 19 Feb 2017  路  18Comments  路  Source: nlohmann/json

The idea is to not create another copy of std::vector since my cbor data start at certain offset.

    static basic_json from_cbor(const std::vector<uint8_t>& v, const size_t startIndex = 0)
    {
        size_t i = startIndex;
        return from_cbor_internal(v, i);
    }
binary formats enhancemenimprovement proposed fix

All 18 comments

Done.

Thanks!

With 22b59693f110b730d41c8f616f82b1fef72c161a, the offset/startIndex parameter was removed as this behavior can be mimicked with an iterator range. For instance, instead of calling

json j = json::from_cbor(vec, 5);

you can write

json j = json::from_cbor({vec.begin()+5, vec.end()});

Are the braces {} intentional?

Yes, I did not find a better way to implicitly create an input adapter from two parameters. And since there is a parameter with a default value following, I cannot use variadic parameters.

(Though I would love to find a solution!)

I didn't go into the details of input_adapter, but what about adding a new overload of from_{cbor,msgpack} that take an iterator range, just like parse?

You could then construct the input_adapter as you did in your example code.

That would be one solution, but then I also have an input adapter to parse from a uint8_t* and a length, and then I would need another family of overloads...

(Of course, the example above could be written as

json j = json::from_cbor(json::input_adapter(vec.begin()+5, vec.end()));

Actually, this is also supported by parse:

auto j = json::parse(vec.begin(), vec.end());
auto j2 = json::parse(vec.data(), vec.data() + vec.size());

Both cases are covered with the same overload. Plus, that would be symmetrical with other parts of the API (e.g. parse as mentioned).

If those kind of methods could share the same API, that would become really intuitive to use IMO.

I know that this is desirable, but I don't really like copy/pasting so many functions...

On the other hand, there are at most two parameters, so I can add two functions for each parser and forward the arguments to the input_adapter constructors.

@theodelrieu Since you are the SFINAE wizard - what do you think of

template<typename A1, typename A2,
         detail::enable_if_t<std::is_constructible<detail::input_adapter, A1, A2>::value, int> = 0>
static basic_json from_cbor(A1&& a1, A2&& a2, const bool strict = true)
{
    return binary_reader(detail::input_adapter(std::forward<A1>(a1), std::forward<A2>(a2))).parse_cbor(strict);
}

Ahah I'm not sure I deserve that title :)

Well that will work, but I think it's quite brittle for some reasons:

  • Too complex for what it does.

  • You have no idea what kind of arguments the function takes. (Not to mention that input_adapter is not exposed, due to the detail namespace)

  • Might break if there is a new ctor to input_adapter (or one less)

I understand that you don't want to copy code, in order to get the same interface than parse, but that's really what we want IMO:

All those functions (parse/from_cbor/msgpack) only take byte containers or equivalent.

Last but not least, I'd rather write boilerplate in the library code than in the client code, all day everyday! Even more when it's just a few lines ;)

I wish I had some arguments that do not translate into "I am too lazy to do the copying...". :-/

I can take care of it :p

Anyway I'd rather not have to rebase my PR (#700) twice a day, so if you want to wait a bit before committing that's fine for me!

I won't touch this one right now. I'm about to finish #593 and then I let the library go.

Was this page helpful?
0 / 5 - 0 ratings