deprecate old removeMember()

include/json/value.h

@@ -392,14 +392,21 @@ Json::Value obj_value(Json::objectValue); // {}
   /// \return the removed Value, or null.
   /// \pre type() is objectValue or nullValue
   /// \post type() is unchanged
+  /// \deprecated
   Value removeMember(const char* key);
   /// Same as removeMember(const char*)
+  /// \deprecated
   Value removeMember(const std::string& key);
+  /** \brief Remove the named map member.
+
+      Update 'removed' iff removed.
+      \return true iff removed (no exceptions)
+  */
+  bool removeMember(const char* key, Value* removed);
   /** \brief Remove the indexed array element.
 
       O(n) expensive operations.
       Update 'removed' iff removed.
-      (This is a better pattern than removeMember().)
       \return true iff removed (no exceptions)
   */
   bool removeIndex(ArrayIndex i, Value* removed);

src/lib_json/json_value.cpp

@@ -989,29 +989,40 @@ Value Value::get(const std::string& key, const Value& defaultValue) const {
   return get(key.c_str(), defaultValue);
 }
 
+
+bool Value::removeMember(const char* key, Value* removed) {
+  if (type_ != objectValue) {
+    return false;
+  }
+#ifndef JSON_VALUE_USE_INTERNAL_MAP
+  CZString actualKey(key, CZString::noDuplication);
+  ObjectValues::iterator it = value_.map_->find(actualKey);
+  if (it == value_.map_->end())
+    return false;
+  *removed = it->second;
+  value_.map_->erase(it);
+  return true;
+#else
+  Value* value = value_.map_->find(key);
+  if (value) {
+    *removed = *value;
+    value_.map_.remove(key);
+    return true;
+  } else {
+    return false;
+  }
+#endif
+}
+
 Value Value::removeMember(const char* key) {
   JSON_ASSERT_MESSAGE(type_ == nullValue || type_ == objectValue,
                       "in Json::Value::removeMember(): requires objectValue");
   if (type_ == nullValue)
     return null;
-#ifndef JSON_VALUE_USE_INTERNAL_MAP
+
-  CZString actualKey(key, CZString::noDuplication);
+  Value removed;  // null
-  ObjectValues::iterator it = value_.map_->find(actualKey);
+  removeMember(key, &removed);
-  if (it == value_.map_->end())
+  return removed; // still null if removeMember() did nothing
-    return null;
-  Value old(it->second);
-  value_.map_->erase(it);
-  return old;
-#else
-  Value* value = value_.map_->find(key);
-  if (value) {
-    Value old(*value);
-    value_.map_.remove(key);
-    return old;
-  } else {
-    return null;
-  }
-#endif
 }
 
 Value Value::removeMember(const std::string& key) {

src/test_lib_json/main.cpp

@@ -202,10 +202,14 @@ JSONTEST_FIXTURE(ValueTest, objects) {
 
   // Remove.
   Json::Value got;
-  got = object1_.removeMember("some other id");
+  bool did;
+  did = object1_.removeMember("some other id", &got);
   JSONTEST_ASSERT_EQUAL(Json::Value("foo"), got);
-  got = object1_.removeMember("some other id");
+  JSONTEST_ASSERT_EQUAL(true, did);
-  JSONTEST_ASSERT_EQUAL(Json::nullValue, got);
+  got = Json::Value("bar");
+  did = object1_.removeMember("some other id", &got);
+  JSONTEST_ASSERT_EQUAL(Json::Value("bar"), got);
+  JSONTEST_ASSERT_EQUAL(false, did);
 }
 
 JSONTEST_FIXTURE(ValueTest, arrays) {