Make JSON methods pass thru to the implementation.
R=roubert@google.com
Review URL: https://codereview.appspot.com/115210043
git-svn-id: http://libaddressinput.googlecode.com/svn/trunk@317 38ededc0-08b8-5190-f2ac-b31f878777ad
diff --git a/cpp/src/preload_supplier.cc b/cpp/src/preload_supplier.cc
index 3a9cdc3..26f7e2b 100644
--- a/cpp/src/preload_supplier.cc
+++ b/cpp/src/preload_supplier.cc
@@ -114,13 +114,13 @@
for (std::vector<std::string>::const_iterator
it = json.GetKeys().begin(); it != json.GetKeys().end(); ++it) {
- if (!json.HasDictionaryValueForKey(*it)) {
+ const Json* value;
+ if (!json.GetDictionaryValueForKey(*it, &value)) {
success = false;
goto callback;
}
- const Json& value = json.GetDictionaryValueForKey(*it);
- if (!value.GetStringValueForKey("id", &id)) {
+ if (!value->GetStringValueForKey("id", &id)) {
success = false;
goto callback;
}
@@ -135,7 +135,7 @@
// All rules on the COUNTRY level inherit from the default rule.
rule->CopyFrom(Rule::GetDefault());
}
- rule->ParseJsonRule(value);
+ rule->ParseJsonRule(*value);
assert(id == rule->GetId()); // Sanity check.
rule_storage_->push_back(rule);
diff --git a/cpp/src/util/json.cc b/cpp/src/util/json.cc
index 4f26069..14a9d11 100644
--- a/cpp/src/util/json.cc
+++ b/cpp/src/util/json.cc
@@ -36,75 +36,97 @@
class Json::JsonImpl {
public:
- // Takes ownership of |document|.
- explicit JsonImpl(const Document* document)
- : document_(document), value_(document), dictionaries_() {
- assert(value_ != NULL);
- assert(value_->IsObject());
- BuildKeyList();
- }
-
- // Does not take ownership of |value|.
- explicit JsonImpl(const Value* value)
- : document_(), value_(value), dictionaries_() {
- assert(value_ != NULL);
- assert(value_->IsObject());
- BuildKeyList();
+ explicit JsonImpl(const std::string& json)
+ : document_(new Document),
+ value_(document_.get()),
+ dictionaries_(),
+ keys_(),
+ valid_(false) {
+ document_->Parse<kParseValidateEncodingFlag>(json.c_str());
+ valid_ = !document_->HasParseError() && document_->IsObject();
+ if (valid_) {
+ BuildKeyList();
+ }
}
~JsonImpl() {
for (std::map<std::string, const Json*>::const_iterator
it = dictionaries_.begin();
- it != dictionaries_.end(); ++it) {
+ it != dictionaries_.end();
+ ++it) {
delete it->second;
}
}
- // The caller does not own the result.
- const Value::Member* FindMember(const std::string& key) {
- return value_->FindMember(key.c_str());
+ bool valid() const { return valid_; }
+
+ const std::vector<std::string>& GetKeys() const { return keys_; }
+
+ bool GetStringValueForKey(const std::string& key, std::string* value) const {
+ assert(value != NULL);
+
+ Value::ConstMemberIterator member = value_->FindMember(key.c_str());
+ if (member == NULL || !member->value.IsString()) {
+ return false;
+ }
+
+ value->assign(member->value.GetString(),
+ member->value.GetStringLength());
+ return true;
}
- // The caller does not own the result. The result can be NULL if there's no
- // dictionary for |key|.
- const Json* FindDictionary(const std::string& key) const {
- std::map<std::string, const Json*>::const_iterator it =
+ bool GetDictionaryValueForKey(const std::string& key, const Json** value) {
+ assert(value != NULL);
+
+ std::map<std::string, const Json*>::const_iterator dict_it =
dictionaries_.find(key);
- return it != dictionaries_.end() ? it->second : NULL;
- }
+ if (dict_it != dictionaries_.end()) {
+ *value = dict_it->second;
+ return true;
+ }
- // Takes ownership of |dictionary|. Should be called only once per |key| and
- // per |dictionary|.
- void AddDictionary(const std::string& key, const Json* dictionary) {
+ Value::ConstMemberIterator member = value_->FindMember(key.c_str());
+ if (member == NULL || !member->value.IsObject()) {
+ return false;
+ }
+
+ Json* sub_dictionary = new Json;
+ sub_dictionary->impl_.reset(new JsonImpl(&member->value));
bool inserted =
- dictionaries_.insert(std::make_pair(key, dictionary)).second;
+ dictionaries_.insert(std::make_pair(key, sub_dictionary)).second;
// Cannot do work inside of assert(), because the compiler can optimize it
// away.
assert(inserted);
// Avoid unused variable warning when assert() is optimized away.
(void)inserted;
+
+ *value = sub_dictionary;
+ return true;
}
- const std::vector<std::string>& GetKeys() const { return keys_; }
-
private:
+ // Does not take ownership of |value|.
+ explicit JsonImpl(const Value* value)
+ : document_(),
+ value_(value),
+ dictionaries_(),
+ keys_(),
+ valid_(true) {
+ assert(value_ != NULL);
+ assert(value_->IsObject());
+ BuildKeyList();
+ }
+
void BuildKeyList() {
assert(keys_.empty());
- for (Value::ConstMemberIterator it = value_->MemberBegin();
- it != value_->MemberEnd(); ++it) {
- keys_.push_back(it->name.GetString());
+ for (Value::ConstMemberIterator member = value_->MemberBegin();
+ member != value_->MemberEnd(); ++member) {
+ keys_.push_back(member->name.GetString());
}
}
// An owned JSON document. Can be NULL if the JSON document is not owned.
- //
- // When a JsonImpl object is constructed using a Document object, then
- // JsonImpl is supposed to take ownership of that object, making sure to
- // delete it in its own destructor. But when a JsonImpl object is constructed
- // using a Value object, then that object is owned by a Member object which is
- // owned by a Document object, and should therefore not be deleted by
- // JsonImpl.
- const scoped_ptr<const Document> document_;
+ const scoped_ptr<Document> document_;
// A JSON document that is not owned. Cannot be NULL. Can point to document_.
const Value* const value_;
@@ -112,8 +134,12 @@
// Owned JSON objects.
std::map<std::string, const Json*> dictionaries_;
+ // The list of keys with values in the JSON object.
std::vector<std::string> keys_;
+ // True if the JSON object was parsed successfully.
+ bool valid_;
+
DISALLOW_COPY_AND_ASSIGN(JsonImpl);
};
@@ -123,13 +149,11 @@
bool Json::ParseObject(const std::string& json) {
assert(impl_ == NULL);
- scoped_ptr<Document> document(new Document);
- document->Parse<kParseValidateEncodingFlag>(json.c_str());
- bool valid = !document->HasParseError() && document->IsObject();
- if (valid) {
- impl_.reset(new JsonImpl(document.release()));
+ impl_.reset(new JsonImpl(json));
+ if (!impl_->valid()) {
+ impl_.reset();
}
- return valid;
+ return impl_ != NULL;
}
const std::vector<std::string>& Json::GetKeys() const {
@@ -140,50 +164,13 @@
bool Json::GetStringValueForKey(const std::string& key,
std::string* value) const {
assert(impl_ != NULL);
- assert(value != NULL);
-
- // Member is owned by impl_.
- const Value::Member* member = impl_->FindMember(key.c_str());
- if (member == NULL || !member->value.IsString()) {
- return false;
- }
-
- value->assign(member->value.GetString(), member->value.GetStringLength());
- return true;
+ return impl_->GetStringValueForKey(key, value);
}
-bool Json::HasDictionaryValueForKey(const std::string& key) const {
+bool Json::GetDictionaryValueForKey(const std::string& key,
+ const Json** value) const {
assert(impl_ != NULL);
-
- // The value returned by FindDictionary() is owned by impl_.
- if (impl_->FindDictionary(key) != NULL) {
- return true;
- }
-
- // Member is owned by impl_.
- const Value::Member* member = impl_->FindMember(key);
- return member != NULL && member->value.IsObject();
-}
-
-const Json& Json::GetDictionaryValueForKey(const std::string& key) const {
- assert(impl_ != NULL);
-
- // Existing_dictionary is owned by impl_.
- const Json* existing_dictionary = impl_->FindDictionary(key);
- if (existing_dictionary != NULL) {
- return *existing_dictionary;
- }
-
- // Member is owned by impl_.
- const Value::Member* member = impl_->FindMember(key);
- assert(member != NULL);
- assert(member->value.IsObject());
-
- // Dictionary is owned by impl_.
- Json* dictionary = new Json;
- dictionary->impl_.reset(new JsonImpl(&member->value));
- impl_->AddDictionary(key, dictionary);
- return *dictionary;
+ return impl_->GetDictionaryValueForKey(key, value);
}
} // namespace addressinput
diff --git a/cpp/src/util/json.h b/cpp/src/util/json.h
index 90565e6..1d66aeb 100644
--- a/cpp/src/util/json.h
+++ b/cpp/src/util/json.h
@@ -49,15 +49,12 @@
// parameter should not be NULL.
bool GetStringValueForKey(const std::string& key, std::string* value) const;
- // Returns true if the parsed JSON contains a dictionary value for |key|. The
- // JSON object must be parsed successfully in ParseObject() before invoking
- // this method.
- bool HasDictionaryValueForKey(const std::string& key) const;
-
- // Returns the dictionary value for the |key|. The |key| must be present and
- // its value must be of the dictionary type, i.e., HasDictionaryValueForKey()
- // must return true before invoking this method.
- const Json& GetDictionaryValueForKey(const std::string& key) const;
+ // Returns true if the parsed JSON contains a dictionary value for |key|.
+ // Points |value| to the dictionary value of the |key|. The JSON object must
+ // be parsed successfully in ParseObject() before invoking this method. The
+ // caller does not own |value|.
+ bool GetDictionaryValueForKey(const std::string& key,
+ const Json** value) const;
private:
class JsonImpl;
diff --git a/cpp/test/util/json_test.cc b/cpp/test/util/json_test.cc
index ff23ea6..2306ac3 100644
--- a/cpp/test/util/json_test.cc
+++ b/cpp/test/util/json_test.cc
@@ -120,16 +120,18 @@
TEST(JsonTest, NoDictionaryFound) {
Json json;
ASSERT_TRUE(json.ParseObject("{\"key\":\"value\"}"));
- EXPECT_FALSE(json.HasDictionaryValueForKey("key"));
+ const Json* sub_json;
+ EXPECT_FALSE(json.GetDictionaryValueForKey("key", &sub_json));
}
TEST(JsonTest, DictionaryFound) {
Json json;
ASSERT_TRUE(json.ParseObject("{\"key\":{\"inner_key\":\"value\"}}"));
- ASSERT_TRUE(json.HasDictionaryValueForKey("key"));
- const Json& sub_json = json.GetDictionaryValueForKey("key");
+ const Json* sub_json = NULL;
+ ASSERT_TRUE(json.GetDictionaryValueForKey("key", &sub_json));
+ ASSERT_TRUE(sub_json != NULL);
std::string value;
- EXPECT_TRUE(sub_json.GetStringValueForKey("inner_key", &value));
+ EXPECT_TRUE(sub_json->GetStringValueForKey("inner_key", &value));
EXPECT_EQ("value", value);
}
@@ -139,10 +141,11 @@
std::vector<std::string> expected_keys(1, "key");
EXPECT_EQ(expected_keys, json.GetKeys());
- ASSERT_TRUE(json.HasDictionaryValueForKey("key"));
- const Json& sub_json = json.GetDictionaryValueForKey("key");
+ const Json* sub_json = NULL;
+ ASSERT_TRUE(json.GetDictionaryValueForKey("key", &sub_json));
+ ASSERT_TRUE(sub_json != NULL);
std::vector<std::string> expected_sub_keys(1, "inner_key");
- EXPECT_EQ(expected_sub_keys, sub_json.GetKeys());
+ EXPECT_EQ(expected_sub_keys, sub_json->GetKeys());
}
} // namespace
