Inconsistent behavior in conversion to array type

[nlohmann]

The library supports the conversion from JSON arrays to STL containers such as std::vector, std::list, etc. Example code would be:

json j = {1, 2, 3, 4};
std::vector<int> v = j;
std::list<int> l = j;
std:: forward_list <int> f = j;

However, the library's behavior is inconsistent when it comes to non-array values:

json j = 12;
std::vector<int> v = j; // throws std::domain_error: type must be array, but is number
std::list<int> l = j; // successfully creates list { 12 };
std::forward_list<int> f = j; // successfully creates list { 12 };

This inconsistent behavior is bad design. There are two possibilities to fix it:

1. Always insist that the JSON value must be an array when a conversion to an STL container of list type is requested. This would mean the second example would produce an exception in all cases.
2. Always create a result, even if it is singleton (in case for booleans, strings, and numbers) or even empty (in case of null).

Internally, we need to iterate over the array in most cases anyway, so we would have no extra costs for solution 2 irregardless of the type. That means: removing the test for an array type would not mean additional costs.

Any thoughts on this?

[theodelrieu]

My first thought is that the solution 2 can lead to a lot of bugs, whereas the solution 1 is consistent and straightforward.

I would go for solution 1, but again this is my first thought

[c650]

I started watching this library yesterday, so I haven't contributed code, but what I'd suggest is...

Solution one seems the most logical for a user, because converting some single JSON value (not an array) to an array doesn't make much sense.

The second solution is more convenient, though, because it allows the users of this library to not have to stick the JSON value into a JSON array/STL container if they wanted to make a single value into an array.

[tksatware]

I'd vote for solution 1 - a scalar type can't be converted to an array/vector type.

[fnc12]

Me too - solution 1. Less implicitness - more clarity

[nlohmann]

So option 1 it is. Thanks for the input!

[nlohmann]

So I wanted to fix this issue and added a regression test:

SECTION("issue #473 - inconsistent behavior in conversion to array type")
{
    json j_array = {1, 2, 3, 4};
    json j_number = 42;
    json j_null = nullptr;

    SECTION("std::vector")
    {
        auto create = [](const json & j)
        {
            std::vector<int> v = j;
        };

        CHECK_NOTHROW(create(j_array));
        CHECK_THROWS_AS(create(j_number), std::domain_error);
        CHECK_THROWS_WITH(create(j_number), "type must be array, but is number");
        CHECK_THROWS_AS(create(j_null), std::domain_error);
        CHECK_THROWS_WITH(create(j_null), "type must be array, but is null");
    }

    SECTION("std::list")
    {
        auto create = [](const json & j)
        {
            std::list<int> v = j;
        };

        CHECK_NOTHROW(create(j_array));
        CHECK_THROWS_AS(create(j_number), std::domain_error);
        CHECK_THROWS_WITH(create(j_number), "type must be array, but is number");
        CHECK_THROWS_AS(create(j_null), std::domain_error);
        CHECK_THROWS_WITH(create(j_null), "type must be array, but is null");
    }

    SECTION("std::forward_list")
    {
        auto create = [](const json & j)
        {
            std::forward_list<int> v = j;
        };

        CHECK_NOTHROW(create(j_array));
        CHECK_THROWS_AS(create(j_number), std::domain_error);
        CHECK_THROWS_WITH(create(j_number), "type must be array, but is number");
        CHECK_THROWS_AS(create(j_null), std::domain_error);
        CHECK_THROWS_WITH(create(j_null), "type must be array, but is null");
    }
}

"Unfortunately", the test passed without me fixing anything.

[nlohmann]

@theodelrieu I would like to ask you some questions on the from_json methods some time. Please let me know when you have time for a brief chat in Slack.

[nlohmann]

With commit 9355f05 I cleaned the array from_json functions to all behave in the same way: An exception is thrown if the JSON value is not an array. If the change is OK with Travis, I shall close this.