commit c4305600835b91630f9ca4b10ad9070ea55a726c
author Vitaly Buka <vitalybuka@google.com> Tue Nov 24 23:33:09 2015 -0800
committer Vitaly Buka <vitalybuka@google.com> Wed Nov 25 17:43:16 2015 +0000
tree dc9246f9de2910540f98fa441f4496a7f103beb1
parent 8a05bebb95f5f3f9bf6b72bd9e58b6ba6b8c9bdd

Replace Get* methods returning unique_ptr with reference alternative

Existing code created temporarily objects and returned them to the
client. It was not efficient and error-prone as client code could
retrieve pointers to internal objects without keeping unique_ptr alive.

Change-Id: I9e17c8d9f66dfc9f52ce9ffc9a31992b16b00461
Reviewed-on: https://weave-review.googlesource.com/1672
Reviewed-by: Alex Vakulenko <avakulenko@google.com>

src/commands/cloud_command_proxy.cc

diff --git a/src/commands/cloud_command_proxy.cc b/src/commands/cloud_command_proxy.cc
index 91db18c..3d472c7 100644
--- a/src/commands/cloud_command_proxy.cc
+++ b/src/commands/cloud_command_proxy.cc
@@ -43,7 +43,7 @@
 void CloudCommandProxy::OnResultsChanged() {
   std::unique_ptr<base::DictionaryValue> patch{new base::DictionaryValue};
   patch->Set(commands::attributes::kCommand_Results,
-             command_instance_->GetResults().release());
+             command_instance_->GetResults().CreateDeepCopy());
   QueueCommandUpdate(std::move(patch));
 }
 
@@ -57,7 +57,7 @@
 void CloudCommandProxy::OnProgressChanged() {
   std::unique_ptr<base::DictionaryValue> patch{new base::DictionaryValue};
   patch->Set(commands::attributes::kCommand_Progress,
-             command_instance_->GetProgress().release());
+             command_instance_->GetProgress().CreateDeepCopy());
   QueueCommandUpdate(std::move(patch));
 }
 

src/commands/command_instance.cc

diff --git a/src/commands/command_instance.cc b/src/commands/command_instance.cc
index aa71b0e..8328299 100644
--- a/src/commands/command_instance.cc
+++ b/src/commands/command_instance.cc
@@ -94,16 +94,16 @@
   return origin_;
 }
 
-std::unique_ptr<base::DictionaryValue> CommandInstance::GetParameters() const {
-  return std::unique_ptr<base::DictionaryValue>(parameters_.DeepCopy());
+const base::DictionaryValue& CommandInstance::GetParameters() const {
+  return parameters_;
 }
 
-std::unique_ptr<base::DictionaryValue> CommandInstance::GetProgress() const {
-  return std::unique_ptr<base::DictionaryValue>(progress_.DeepCopy());
+const base::DictionaryValue& CommandInstance::GetProgress() const {
+  return progress_;
 }
 
-std::unique_ptr<base::DictionaryValue> CommandInstance::GetResults() const {
-  return std::unique_ptr<base::DictionaryValue>(results_.DeepCopy());
+const base::DictionaryValue& CommandInstance::GetResults() const {
+  return results_;
 }
 
 const Error* CommandInstance::GetError() const {

src/commands/command_instance.h

diff --git a/src/commands/command_instance.h b/src/commands/command_instance.h
index e7a6eca..60620a1 100644
--- a/src/commands/command_instance.h
+++ b/src/commands/command_instance.h
@@ -54,9 +54,9 @@
   const std::string& GetName() const override;
   Command::State GetState() const override;
   Command::Origin GetOrigin() const override;
-  std::unique_ptr<base::DictionaryValue> GetParameters() const override;
-  std::unique_ptr<base::DictionaryValue> GetProgress() const override;
-  std::unique_ptr<base::DictionaryValue> GetResults() const override;
+  const base::DictionaryValue& GetParameters() const override;
+  const base::DictionaryValue& GetProgress() const override;
+  const base::DictionaryValue& GetResults() const override;
   const Error* GetError() const override;
   bool SetProgress(const base::DictionaryValue& progress,
                    ErrorPtr* error) override;

src/commands/command_instance_unittest.cc

diff --git a/src/commands/command_instance_unittest.cc b/src/commands/command_instance_unittest.cc
index 1d32cd9..fb8fe84 100644
--- a/src/commands/command_instance_unittest.cc
+++ b/src/commands/command_instance_unittest.cc
@@ -82,8 +82,8 @@
   EXPECT_EQ("robot.speak", instance.GetName());
   EXPECT_EQ(Command::Origin::kCloud, instance.GetOrigin());
   EXPECT_JSON_EQ("{'phrase': 'iPityDaFool', 'volume': 5}",
-                 *instance.GetParameters());
-  EXPECT_JSON_EQ("{'foo': 239}", *instance.GetResults());
+                 instance.GetParameters());
+  EXPECT_JSON_EQ("{'foo': 239}", instance.GetResults());
 
   CommandInstance instance2{"base.reboot",
                             Command::Origin::kLocal,
@@ -118,7 +118,7 @@
   EXPECT_EQ("abcd", instance->GetID());
   EXPECT_EQ("robot.jump", instance->GetName());
   EXPECT_JSON_EQ("{'height': 53, '_jumpType': '_withKick'}",
-                 *instance->GetParameters());
+                 instance->GetParameters());
 }
 
 TEST_F(CommandInstanceTest, FromJson_ParamsOmitted) {
@@ -126,7 +126,7 @@
   auto instance = CommandInstance::FromJson(json.get(), Command::Origin::kCloud,
                                             dict_, nullptr, nullptr);
   EXPECT_EQ("base.reboot", instance->GetName());
-  EXPECT_JSON_EQ("{}", *instance->GetParameters());
+  EXPECT_JSON_EQ("{}", instance->GetParameters());
 }
 
 TEST_F(CommandInstanceTest, FromJson_NotObject) {