Avoid string copying in json.cc.
The following two methods:
bool Json::HasStringValueForKey(const std::string& key) const;
std::string Json::GetStringValueForKey(const std::string& key) const;
have been replaced with a single method:
bool Json::GetStringValueForKey(const std::string& key,
std::string* value) const;
to avoid copying possibly large strings.
BUG=8
R=roubert@google.com
Review URL: https://codereview.appspot.com/112940044
git-svn-id: http://libaddressinput.googlecode.com/svn/trunk@303 38ededc0-08b8-5190-f2ac-b31f878777ad
diff --git a/cpp/src/preload_supplier.cc b/cpp/src/preload_supplier.cc
index 31325bd..c613d3c 100644
--- a/cpp/src/preload_supplier.cc
+++ b/cpp/src/preload_supplier.cc
@@ -105,6 +105,7 @@
(void)status; // Prevent unused variable if assert() is optimized away.
Json json;
+ std::string id;
std::vector<const Rule*> sub_rules;
if (!success) {
@@ -124,11 +125,10 @@
}
const Json& value = json.GetDictionaryValueForKey(*it);
- if (!value.HasStringValueForKey("id")) {
+ if (!value.GetStringValueForKey("id", &id)) {
success = false;
goto callback;
}
- const std::string& id = value.GetStringValueForKey("id");
assert(*it == id); // Sanity check.
size_t depth = std::count(id.begin(), id.end(), '/') - 1;
diff --git a/cpp/src/rule.cc b/cpp/src/rule.cc
index b4d03ca..4d1cff5 100644
--- a/cpp/src/rule.cc
+++ b/cpp/src/rule.cc
@@ -38,20 +38,6 @@
typedef std::map<std::string, int> NameMessageIdMap;
-const char kAdminAreaNameTypeKey[] = "state_name_type";
-const char kFormatKey[] = "fmt";
-const char kIdKey[] = "id";
-const char kLanguagesKey[] = "languages";
-const char kLatinFormatKey[] = "lfmt";
-const char kLatinNameKey[] = "lname";
-const char kNameKey[] = "name";
-const char kPostalCodeNameTypeKey[] = "zip_name_type";
-const char kRequireKey[] = "require";
-const char kSubKeysKey[] = "sub_keys";
-const char kZipKey[] = "zip";
-const char kPostalCodeExampleKey[] = "zipex";
-const char kPostServiceUrlKey[] = "posturl";
-
// Used as a separator in a list of items. For example, the list of supported
// languages can be "de~fr~it".
const char kSeparator = '~';
@@ -182,36 +168,33 @@
}
void Rule::ParseJsonRule(const Json& json) {
- if (json.HasStringValueForKey(kIdKey)) {
- id_ = json.GetStringValueForKey(kIdKey);
+ std::string value;
+ if (json.GetStringValueForKey("id", &value)) {
+ id_.swap(value);
}
- if (json.HasStringValueForKey(kFormatKey)) {
- ParseFormatRule(json.GetStringValueForKey(kFormatKey), &format_);
+ if (json.GetStringValueForKey("fmt", &value)) {
+ ParseFormatRule(value, &format_);
}
- if (json.HasStringValueForKey(kLatinFormatKey)) {
- ParseFormatRule(json.GetStringValueForKey(kLatinFormatKey), &latin_format_);
+ if (json.GetStringValueForKey("lfmt", &value)) {
+ ParseFormatRule(value, &latin_format_);
}
- if (json.HasStringValueForKey(kRequireKey)) {
- ParseAddressFieldsRequired(
- json.GetStringValueForKey(kRequireKey), &required_);
+ if (json.GetStringValueForKey("require", &value)) {
+ ParseAddressFieldsRequired(value, &required_);
}
- if (json.HasStringValueForKey(kSubKeysKey)) {
- SplitString(
- json.GetStringValueForKey(kSubKeysKey), kSeparator, &sub_keys_);
+ if (json.GetStringValueForKey("sub_keys", &value)) {
+ SplitString(value, kSeparator, &sub_keys_);
}
- if (json.HasStringValueForKey(kLanguagesKey)) {
- SplitString(
- json.GetStringValueForKey(kLanguagesKey), kSeparator, &languages_);
+ if (json.GetStringValueForKey("languages", &value)) {
+ SplitString(value, kSeparator, &languages_);
}
sole_postal_code_.clear();
- if (json.HasStringValueForKey(kZipKey)) {
- const std::string& zip = json.GetStringValueForKey(kZipKey);
+ if (json.GetStringValueForKey("zip", &value)) {
// The "zip" field in the JSON data is used in two different ways to
// validate the postal code. At the country level, the "zip" field indicates
// a Java compatible regular expression corresponding to all postal codes in
@@ -225,7 +208,7 @@
// RE2::FullMatch() to perform matching against the entire string.
RE2::Options options;
options.set_never_capture(true);
- RE2* matcher = new RE2("^(" + zip + ")", options);
+ RE2* matcher = new RE2("^(" + value + ")", options);
if (matcher->ok()) {
postal_code_matcher_.reset(new RE2ptr(matcher));
} else {
@@ -234,37 +217,35 @@
}
// If the "zip" field is not a regular expression, then it is the sole
// postal code for this rule.
- if (!ContainsRegExSpecialCharacters(zip)) {
- sole_postal_code_ = zip;
+ if (!ContainsRegExSpecialCharacters(value)) {
+ sole_postal_code_.swap(value);
}
}
- if (json.HasStringValueForKey(kAdminAreaNameTypeKey)) {
+ if (json.GetStringValueForKey("state_name_type", &value)) {
admin_area_name_message_id_ =
- GetMessageIdFromName(json.GetStringValueForKey(kAdminAreaNameTypeKey),
- GetAdminAreaMessageIds());
+ GetMessageIdFromName(value, GetAdminAreaMessageIds());
}
- if (json.HasStringValueForKey(kPostalCodeNameTypeKey)) {
+ if (json.GetStringValueForKey("zip_name_type", &value)) {
postal_code_name_message_id_ =
- GetMessageIdFromName(json.GetStringValueForKey(kPostalCodeNameTypeKey),
- GetPostalCodeMessageIds());
+ GetMessageIdFromName(value, GetPostalCodeMessageIds());
}
- if (json.HasStringValueForKey(kNameKey)) {
- name_ = json.GetStringValueForKey(kNameKey);
+ if (json.GetStringValueForKey("name", &value)) {
+ name_.swap(value);
}
- if (json.HasStringValueForKey(kLatinNameKey)) {
- latin_name_ = json.GetStringValueForKey(kLatinNameKey);
+ if (json.GetStringValueForKey("lname", &value)) {
+ latin_name_.swap(value);
}
- if (json.HasStringValueForKey(kPostalCodeExampleKey)) {
- postal_code_example_ = json.GetStringValueForKey(kPostalCodeExampleKey);
+ if (json.GetStringValueForKey("zipex", &value)) {
+ postal_code_example_.swap(value);
}
- if (json.HasStringValueForKey(kPostServiceUrlKey)) {
- post_service_url_ = json.GetStringValueForKey(kPostServiceUrlKey);
+ if (json.GetStringValueForKey("posturl", &value)) {
+ post_service_url_.swap(value);
}
}
diff --git a/cpp/src/util/json.cc b/cpp/src/util/json.cc
index d819bb2..352c62b 100644
--- a/cpp/src/util/json.cc
+++ b/cpp/src/util/json.cc
@@ -139,23 +139,19 @@
return impl_->GetKeys();
}
-bool Json::HasStringValueForKey(const std::string& key) const {
+bool Json::GetStringValueForKey(const std::string& key,
+ std::string* value) const {
assert(impl_ != NULL);
-
- // Member is owned by impl_.
- const Value::Member* member = impl_->FindMember(key);
- return member != NULL && member->value.IsString();
-}
-
-std::string Json::GetStringValueForKey(const std::string& key) const {
- assert(impl_ != NULL);
+ assert(value != NULL);
// Member is owned by impl_.
const Value::Member* member = impl_->FindMember(key.c_str());
- assert(member != NULL);
- assert(member->value.IsString());
- return std::string(member->value.GetString(),
- member->value.GetStringLength());
+ if (member == NULL || !member->value.IsString()) {
+ return false;
+ }
+
+ value->assign(member->value.GetString(), member->value.GetStringLength());
+ return true;
}
bool Json::HasDictionaryValueForKey(const std::string& key) const {
diff --git a/cpp/src/util/json.h b/cpp/src/util/json.h
index 1fbf500..90565e6 100644
--- a/cpp/src/util/json.h
+++ b/cpp/src/util/json.h
@@ -43,15 +43,11 @@
// successfully in ParseObject() before invoking this method.
const std::vector<std::string>& GetKeys() const;
- // Returns true if the parsed JSON contains a string value for |key|. The JSON
- // object must be parsed successfully in ParseObject() before invoking this
- // method.
- bool HasStringValueForKey(const std::string& key) const;
-
- // Returns the string value for the |key|. The |key| must be present and its
- // value must be of string type, i.e., HasStringValueForKey(key) must return
- // true before invoking this method.
- std::string GetStringValueForKey(const std::string& key) const;
+ // Returns true if the parsed JSON contains a string value for |key|. Sets
+ // |value| to the string value of the |key|. The JSON object must be parsed
+ // successfully in ParseObject() before invoking this method. The |value|
+ // 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
diff --git a/cpp/test/util/json_test.cc b/cpp/test/util/json_test.cc
index f6eb389..a3b9f80 100644
--- a/cpp/test/util/json_test.cc
+++ b/cpp/test/util/json_test.cc
@@ -32,8 +32,9 @@
TEST(JsonTest, EmptyDictionaryContainsNoKeys) {
Json json;
ASSERT_TRUE(json.ParseObject("{}"));
- EXPECT_FALSE(json.HasStringValueForKey("key"));
- EXPECT_FALSE(json.HasStringValueForKey(std::string()));
+ std::string not_checked;
+ EXPECT_FALSE(json.GetStringValueForKey("key", ¬_checked));
+ EXPECT_FALSE(json.GetStringValueForKey(std::string(), ¬_checked));
}
TEST(JsonTest, InvalidJsonIsNotValid) {
@@ -44,35 +45,40 @@
TEST(JsonTest, OneKeyIsValid) {
Json json;
ASSERT_TRUE(json.ParseObject("{\"key\": \"value\"}"));
- ASSERT_TRUE(json.HasStringValueForKey("key"));
- EXPECT_EQ("value", json.GetStringValueForKey("key"));
+ std::string value;
+ EXPECT_TRUE(json.GetStringValueForKey("key", &value));
+ EXPECT_EQ("value", value);
}
TEST(JsonTest, EmptyStringKeyIsNotInObject) {
Json json;
ASSERT_TRUE(json.ParseObject("{\"key\": \"value\"}"));
- EXPECT_FALSE(json.HasStringValueForKey(std::string()));
+ std::string not_checked;
+ EXPECT_FALSE(json.GetStringValueForKey(std::string(), ¬_checked));
}
TEST(JsonTest, EmptyKeyIsValid) {
Json json;
ASSERT_TRUE(json.ParseObject("{\"\": \"value\"}"));
- ASSERT_TRUE(json.HasStringValueForKey(std::string()));
- EXPECT_EQ("value", json.GetStringValueForKey(std::string()));
+ std::string value;
+ EXPECT_TRUE(json.GetStringValueForKey(std::string(), &value));
+ EXPECT_EQ("value", value);
}
TEST(JsonTest, EmptyValueIsValid) {
Json json;
ASSERT_TRUE(json.ParseObject("{\"key\": \"\"}"));
- ASSERT_TRUE(json.HasStringValueForKey("key"));
- EXPECT_TRUE(json.GetStringValueForKey("key").empty());
+ std::string value;
+ EXPECT_TRUE(json.GetStringValueForKey("key", &value));
+ EXPECT_TRUE(value.empty());
}
TEST(JsonTest, Utf8EncodingIsValid) {
Json json;
ASSERT_TRUE(json.ParseObject("{\"key\": \"Ü\"}"));
- ASSERT_TRUE(json.HasStringValueForKey("key"));
- EXPECT_EQ("Ü", json.GetStringValueForKey("key"));
+ std::string value;
+ EXPECT_TRUE(json.GetStringValueForKey("key", &value));
+ EXPECT_EQ("Ü", value);
}
TEST(JsonTest, InvalidUtf8IsNotValid) {
@@ -89,11 +95,12 @@
TEST(JsonTest, TwoKeysAreValid) {
Json json;
ASSERT_TRUE(json.ParseObject("{\"key1\": \"value1\", \"key2\": \"value2\"}"));
- ASSERT_TRUE(json.HasStringValueForKey("key1"));
- EXPECT_EQ("value1", json.GetStringValueForKey("key1"));
+ std::string value;
+ EXPECT_TRUE(json.GetStringValueForKey("key1", &value));
+ EXPECT_EQ("value1", value);
- ASSERT_TRUE(json.HasStringValueForKey("key2"));
- EXPECT_EQ("value2", json.GetStringValueForKey("key2"));
+ EXPECT_TRUE(json.GetStringValueForKey("key2", &value));
+ EXPECT_EQ("value2", value);
}
TEST(JsonTest, ListIsNotValid) {
@@ -122,8 +129,9 @@
ASSERT_TRUE(json.ParseObject("{\"key\":{\"inner_key\":\"value\"}}"));
ASSERT_TRUE(json.HasDictionaryValueForKey("key"));
const Json& sub_json = json.GetDictionaryValueForKey("key");
- ASSERT_TRUE(sub_json.HasStringValueForKey("inner_key"));
- EXPECT_EQ("value", sub_json.GetStringValueForKey("inner_key"));
+ std::string value;
+ EXPECT_TRUE(sub_json.GetStringValueForKey("inner_key", &value));
+ EXPECT_EQ("value", value);
}
TEST(JsonTest, DictionariesHaveKeys) {
