С++ rapidjson: GenericValue::IsNull возвращает false в любом случае

Я до сих пор в шоке, обнаружив загадочную проблему в нашем проекте.

Мы поняли, что вызов HasMember("string") выполнял дополнительный поиск. Итак, из соображений производительности мы меняем его.

Основная идея:

Вместо вызова HasMember и последующего предварительного кэширования ссылки, например:

rapidjson::Document d;
d.Parse<0>(json);

if(d.HasMember("foo"))
{
    const rapidjson::Value& fooValue = d["foo"];

    // do something with fooValue
}

Изменился на:

rapidjson::Document d;
d.Parse<0>(json);

const rapidjson::Value& fooValue = d["foo"];
if( !fooValue.IsNull() )
{
    // do something with fooValue
}

Это было довольно хорошо, мы экономим на выполнении двух поисков вместо одного. Однако здесь возникает проблема.

Если вы начнете смотреть, как Rapidjson реализует нулевое значение (возвращается по умолчанию при сбое поиска), вы увидите следующий код:

//! Get the value associated with the object's name.
GenericValue & operator[](const Ch* name) {
    // Check
    if (Member * member = FindMember(name)) {
        return member->value;
    } else {
        // Nothing
        static GenericValue NullValue;
        return NullValue;
    }
}

// Finder
const GenericValue & operator[] (const Ch* name) const { 
    // Return
    return const_cast<GenericValue &> (* this)[name]; 
}

Итак, если член не найден, мы возвращаем локальную статическую переменную. На первый взгляд это может показаться достаточно хорошим, но, поскольку это возвращается по ссылке, это может легко привести к скрытым ошибкам.

Представьте, что кто-то изменил ссылку на статическое значение NullValue. Это приведет к тому, что все дальнейшие вызовы IsNull (после его поиска) завершатся неудачно, потому что NullValue изменился на другой тип или даже на случайную память.

Итак, что вы думаете? Как вы думаете, это хороший пример нулевого шаблона?

Я в замешательстве, мне нравится идея возврата значения null по умолчанию, но поскольку оно не возвращается как const, это опасно. И, даже если мы вернем его во всех случаях как const, разработчики все равно могут использовать const_cast (но я бы не ожидал, что если они это сделают, то будут под их ответственностью).

Я хочу услышать другие случаи и примеры, подобные этому. И если кто-то может дать реальное решение в коде rapidjson, это будет просто потрясающе и потрясающе.


person dmayola    schedule 06.10.2014    source источник
comment
if( !d.IsNull() ) должно быть if( !fooValue.IsNull() ) правильно?   -  person Narek    schedule 24.03.2015
comment
Верно! Отредактировано, спасибо   -  person dmayola    schedule 25.03.2015


Ответы (1)


Ловушка этого дизайна была поднята сообществом давно. Поскольку operator[] также нужна неконстантная версия, невозможно сохранить целостность статической переменной.

Таким образом, этот API был изменен в более новых версиях RapidJSON. operator[] просто утверждает несуществующий ключ. Если нет уверенности в том, что ключ существует, предпочтительно использовать

MemberIterator FindMember(const Ch* name);
ConstMemberIterator FindMember(const Ch* name) const;

И сравнивая значение с MemberEnd(), чтобы проверить, существует ли ключ. Это также задокументировано здесь.

Кроме того, обратите внимание, что RapidJSON был перемещен на GitHub. Многие вопросы решены. Пожалуйста, используйте самую новую версию, если это возможно. Спасибо.

P.S. Я автор RapidJSON.

person Milo Yip    schedule 08.10.2014
comment
Milo, где я могу найти версию rapidjson 0.2? - person Narek; 24.03.2015