From f3398eab283acfacb1e10dc48e07c0cc21cd3599 Mon Sep 17 00:00:00 2001
From: LaGrunge <leontodys@gmail.com>
Date: Thu, 17 Oct 2019 13:05:19 +0300
Subject: [PATCH] Review fixes, pointer instead of reference to distinguish
 null and absent

---
 coding/json.hpp        | 29 ++++++++++-------------------
 geocoder/hierarchy.cpp | 14 ++++++++------
 2 files changed, 18 insertions(+), 25 deletions(-)

diff --git a/coding/json.hpp b/coding/json.hpp
index de7cb49..1f4b8b1 100644
--- a/coding/json.hpp
+++ b/coding/json.hpp
@@ -14,6 +14,7 @@
 #include <iomanip>
 #include <sstream>
 
+
 namespace coding
 {
 DECLARE_EXCEPTION(JsonException, RootException);
@@ -22,14 +23,6 @@ using JsonValue = rapidjson::Value;
 using JsonDocument = rapidjson::Document;
 using JsonParseResult = rapidjson::ParseResult;
 
-template <typename T>
-inline T FromJson(JsonDocument const & root)
-{
-  T result{};
-  FromJson(root, result);
-  return result;
-}
-
 inline void FromJson(JsonValue const & root, double & result)
 {
   if (!root.IsNumber())
@@ -51,9 +44,7 @@ inline void FromJson(JsonValue const & root, std::string & result)
   result = root.GetString();
 }
 
-static const coding::JsonValue nullValue;
-
-inline coding::JsonValue const & GetJsonOptionalField(coding::JsonValue const & root,
+inline coding::JsonValue const * GetJsonOptionalField(coding::JsonValue const & root,
                                                       std::string const & field)
 {
   if (!root.IsObject())
@@ -62,32 +53,32 @@ inline coding::JsonValue const & GetJsonOptionalField(coding::JsonValue const &
   coding::JsonValue::ConstMemberIterator it = root.FindMember(field);
 
   if (it == root.MemberEnd())
-    return nullValue;
+    return nullptr;
 
-  return it->value;
+  return &it->value;
 }
 
 inline coding::JsonValue const & GetJsonObligatoryField(coding::JsonValue const & root,
                                                         std::string const & field)
 {
-  coding::JsonValue const & value = GetJsonOptionalField(root, field);
-  if (value.IsNull())
+  coding::JsonValue const * value = GetJsonOptionalField(root, field);
+  if (!value)
     MYTHROW(coding::JsonException, ("Obligatory field", field, "is absent."));
 
-  return value;
+  return *value;
 }
 
 template <typename T>
 void FromJsonObjectOptionalField(JsonValue const & root, std::string const & field, T & result)
 {
-  coding::JsonValue const & value = GetJsonOptionalField(root, field);
+  coding::JsonValue const * value = GetJsonOptionalField(root, field);
 
-  if (value.IsNull())
+  if (!value)
   {
     result = T{};
     return;
   }
-  FromJson(value, result);
+  FromJson(*value, result);
 }
 
 template <class First>
diff --git a/geocoder/hierarchy.cpp b/geocoder/hierarchy.cpp
index a50fb7b..74cff3d 100644
--- a/geocoder/hierarchy.cpp
+++ b/geocoder/hierarchy.cpp
@@ -86,11 +86,10 @@ bool Hierarchy::Entry::DeserializeAddressFromJSON(
       m_type = static_cast<Type>(i);
     }
   }
-  coding::JsonValue const & rank = coding::GetJsonOptionalField(properties, "rank");
 
-  if (!rank.IsNull())
+  if (coding::JsonValue const * rank = coding::GetJsonOptionalField(properties, "rank"))
   {
-    auto const type = RankToType(rank.GetInt());
+    auto const type = RankToType(rank->GetInt());
     if (type != Type::Count &&
         m_normalizedAddress[static_cast<size_t>(type)] != NameDictionary::kUnspecifiedPosition)
     {
@@ -127,13 +126,16 @@ bool Hierarchy::Entry::FetchAddressFieldNames(coding::JsonValue const & locales,
        itr != locales.MemberEnd(); ++itr)
   {
     coding::JsonValue const & address = coding::GetJsonObligatoryField(itr->value, "address");
-    coding::JsonValue const & levelJson = coding::GetJsonOptionalField(address, levelKey);
+    coding::JsonValue const * levelJson = coding::GetJsonOptionalField(address, levelKey);
 
-    if (levelJson.IsNull())
+    if (!levelJson)
       continue;
 
+    if (levelJson->IsNull())
+      return false;
+
     std::string levelValue;
-    coding::FromJson(levelJson, levelValue);
+    coding::FromJson(*levelJson, levelValue);
     if (levelValue.empty())
       continue;