Conversion with default values

[jpetso]

msgpack::object provides as<>() and convert(T) functions to extract values. In order to handle fields that are potentially nil, newer msgpack revisions also provide support for boost::optional, so I can write code like this:

auto myint = object.as<boost::optional<int>>();
if (myint) {
    // Perform code based on myint existing.
}

There is still an unnecessary verbosity when dealing with default values:

std::map<std::string, whatever_t> mymap;
if (!object.is_nil()) {
    object.convert(mymap);
}
int myint = DEFAULT_NUMBER;
if (!object.is_nil()) {
    object.convert(myint);
}
// Or a slightly more verbose example:
std::vector<int> list = foo->number_list();
if (!object.is_nil()) {
    auto list = object.convert(list);
    foo->set_number_list(list);
}

I can see two potential accessor method patterns that can help simplify this code:

// get_or() (named after boost::optional::get_or()) provides a fallback in case the object is nil.
auto mystring = object.get_or(std::string());
auto mymap = object.get_or(std::map<std::string, whatever_t>());

// For implicit int/uint conversions, get_or() can be dangerous if the caller forgets to also provide the template argument manually:
uint64_t counter1 = object.get_or(0);
// The above resolves to int as template argument, which eliminates much of the uint64_t range.

// The other approach resembles convert(), and lets the user do the initialization of default values explicitly:
uint64_t counter2 = 0;
object.convert_if_not_nil(counter2);

// Having a bool return value for convert_if_not_nil() also helps to reduce the number of lines:
std::vector<int> list = foo->number_list();
if (object.convert_if_not_nil(list)) {
    foo->set_number_list(list);
}

All of the above have in common that object is only used once, which can sometimes help prevent creation of a temporary msgpack::object variable and just continue from a msgpack::object function result instead. Which can make the code even more compact (while retaining readability) compared to the baseline.

While the get_or() suffers from possible template mismatches and can't be used for if (...) foo->set(...); style clauses, convert_if_not_nil() handles both well and seems like an improvement without drawbacks. Here is an implementation (note that it will still throw if the object exists but contains an unexpected type):

template <typename T>
inline bool object::convert_if_not_nil(T& v) const
{
    if (is_nil()) {
        return false;
    }
    v = as<T>();
    return true;
}

How would you feel about this method being added to msgpack::object?

[redboltz]

Thanks you for sending the issue. I think that convert_if_not_nil() is a good idea.

However, I don't understand yet which case is Boost.Optional is not good enough. We can use 'as' with boost::optional<any_types_that_msgpack_supported>.

#include <msgpack.hpp>
#include <iostream>

int main() {
    {
        std::map<std::string, int> m;
        m.emplace("ABC", 42);
        msgpack::zone z;
        msgpack::object o(m, z);
        auto opt = o.as<boost::optional<std::map<std::string, int>>>();
        if (opt) {
            std::cout << "optional has a value." << std::endl;
            auto it = opt->find("ABC");
            if (it != opt->end()){
                std::cout << it->first << ":" << it->second << std::endl;
            }
        }
    }
    {
        msgpack::object o;
        auto opt = o.as<boost::optional<std::map<std::string, int>>>();
        if (opt) {
        }
        else {
            std::cout << "optional doesn't have a value." << std::endl;
        }
    }
}

Output

optional has a value.
ABC:42
optional doesn't have a value.

Do you mean even if we can't use Boost libraries, we can use convert_if_not_nil() instead of that?

[jpetso]

Do you mean even if we can't use Boost libraries, we can use convert_if_not_nil() instead of that?

That would be one way to see it. I think that boost::optional will always provide a feasible way to achieve the desired outcome. I think, however, that it's overly verbose and not overly practical when the non-optional type should be kept after unpacking. In addition to a Boost-less environment, these are the use cases where I believe boost::optional isn't the optimal solution:

• When the msgpack null value doesn't correspond to an empty boost::optional but to a value within the range of the actual type (i.e. empty map/vector, number with default value, enum with none/unknown value).
• When there is an existing value of non-boost::optional type that needs to be worked with, either because it's owned by another object (e.g. struct member, out parameter) or because after deserialization no optionality is required anymore so we don't want an extra boost::optional wrapper.

In such cases, boost::optional will generally require one or two more lines of code than the equivalent using convert_if_not_null(). (Try rephrasing my last example box with boost::optional if you want to check, with a requirement of keeping a non-optional type after unpacking.) In addition to saving lines of code, unpacking directly into the target object also saves one copy or move (instead of object -> boost::optional -> target).

So I while think it's mostly a convenience method rather than a necessity, I think it is one that benefits readability and compactness for common use cases.

[jpetso]

Possible alternative names for convert_if_not_nil(): convert_if_exists(), convert_if_has_value(). (The naming is a trade-off here: I think convert_if_not_nil() describes most accurately what it's doing, but the other two phrase it without negation which is also a good thing. I'm undecided which one is a better trade-off.)

[redboltz]

Thanks for the clarification. I understand. I think convert_if_not_nil is fine too :)