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>

4 files changed, 11 insertions, 24 deletions

diff --git a/include/weave/command.h b/include/weave/command.h
index 59a9305..08ea782 100644
--- a/include/weave/command.h
+++ b/include/weave/command.h
@@ -40,13 +40,13 @@ class Command {
   virtual Command::Origin GetOrigin() const = 0;
 
   // Returns the command parameters.
-  virtual std::unique_ptr<base::DictionaryValue> GetParameters() const = 0;
+  virtual const base::DictionaryValue& GetParameters() const = 0;
 
   // Returns the command progress.
-  virtual std::unique_ptr<base::DictionaryValue> GetProgress() const = 0;
+  virtual const base::DictionaryValue& GetProgress() const = 0;
 
   // Returns the command results.
-  virtual std::unique_ptr<base::DictionaryValue> GetResults() const = 0;
+  virtual const base::DictionaryValue& GetResults() const = 0;
 
   // Returns the command error.
   virtual const Error* GetError() const = 0;
diff --git a/include/weave/device.h b/include/weave/device.h
index 19012b5..cbcc193 100644
--- a/include/weave/device.h
+++ b/include/weave/device.h
@@ -96,7 +96,7 @@ class Device {
 
   // Returns value of the single property.
   // |name| is full property name, including package name. e.g. "base.network".
-  virtual std::unique_ptr<base::Value> GetStateProperty(
+  virtual const base::Value* GetStateProperty(
       const std::string& name) const = 0;
 
   // Sets value of the single property.
@@ -106,7 +106,7 @@ class Device {
                                 ErrorPtr* error) = 0;
 
   // Returns aggregated state properties across all registered packages.
-  virtual std::unique_ptr<base::DictionaryValue> GetState() const = 0;
+  virtual const base::DictionaryValue& GetState() const = 0;
 
   // Returns current state of GCD connection.
   virtual GcdState GetGcdState() const = 0;
diff --git a/include/weave/test/mock_command.h b/include/weave/test/mock_command.h
index 2b1080e..fe1a02a 100644
--- a/include/weave/test/mock_command.h
+++ b/include/weave/test/mock_command.h
@@ -25,9 +25,9 @@ class MockCommand : public Command {
   MOCK_CONST_METHOD0(GetCategory, const std::string&());
   MOCK_CONST_METHOD0(GetState, Command::State());
   MOCK_CONST_METHOD0(GetOrigin, Command::Origin());
-  MOCK_CONST_METHOD0(MockGetParameters, const std::string&());
-  MOCK_CONST_METHOD0(MockGetProgress, const std::string&());
-  MOCK_CONST_METHOD0(MockGetResults, const std::string&());
+  MOCK_CONST_METHOD0(GetParameters, const base::DictionaryValue&());
+  MOCK_CONST_METHOD0(GetProgress, const base::DictionaryValue&());
+  MOCK_CONST_METHOD0(GetResults, const base::DictionaryValue&());
   MOCK_CONST_METHOD0(GetError, const Error*());
   MOCK_METHOD2(SetProgress, bool(const base::DictionaryValue&, ErrorPtr*));
   MOCK_METHOD2(Complete, bool(const base::DictionaryValue&, ErrorPtr*));
@@ -35,10 +35,6 @@ class MockCommand : public Command {
   MOCK_METHOD2(SetError, bool(const Error*, ErrorPtr*));
   MOCK_METHOD2(Abort, bool(const Error*, ErrorPtr*));
   MOCK_METHOD1(Cancel, bool(ErrorPtr*));
-
-  std::unique_ptr<base::DictionaryValue> GetParameters() const override;
-  std::unique_ptr<base::DictionaryValue> GetProgress() const override;
-  std::unique_ptr<base::DictionaryValue> GetResults() const override;
 };
 
 }  // namespace test
diff --git a/include/weave/test/mock_device.h b/include/weave/test/mock_device.h
index f751f97..e5063e7 100644
--- a/include/weave/test/mock_device.h
+++ b/include/weave/test/mock_device.h
@@ -34,13 +34,13 @@ class MockDevice : public Device {
   MOCK_METHOD2(SetStatePropertiesFromJson, bool(const std::string&, ErrorPtr*));
   MOCK_METHOD2(SetStateProperties,
                bool(const base::DictionaryValue&, ErrorPtr*));
-  MOCK_CONST_METHOD1(MockGetStateProperty,
-                     base::Value*(const std::string& name));
+  MOCK_CONST_METHOD1(GetStateProperty,
+                     const base::Value*(const std::string& name));
   MOCK_METHOD3(SetStateProperty,
                bool(const std::string& name,
                     const base::Value& value,
                     ErrorPtr* error));
-  MOCK_CONST_METHOD0(MockGetState, base::DictionaryValue*());
+  MOCK_CONST_METHOD0(GetState, const base::DictionaryValue&());
   MOCK_CONST_METHOD0(GetGcdState, GcdState());
   MOCK_METHOD1(AddGcdStateChangedCallback,
                void(const GcdStateChangedCallback& callback));
@@ -50,15 +50,6 @@ class MockDevice : public Device {
   MOCK_METHOD2(AddPairingChangedCallbacks,
                void(const PairingBeginCallback& begin_callback,
                     const PairingEndCallback& end_callback));
-
-  // Gmock 1.7.0 does not work with unuque_ptr as return value.
-  std::unique_ptr<base::Value> GetStateProperty(
-      const std::string& name) const override {
-    return std::unique_ptr<base::Value>(MockGetStateProperty(name));
-  }
-  std::unique_ptr<base::DictionaryValue> GetState() const override {
-    return std::unique_ptr<base::DictionaryValue>(MockGetState());
-  }
 };
 
 }  // namespace test