fix review comments

open-telemetry · Sep 1, 2020 · aa8ee7c · aa8ee7c
1 parent c648f9f
commit aa8ee7c
Show file tree

Hide file tree

Showing 2 changed files with 21 additions and 19 deletions.
diff --git a/api/include/opentelemetry/http/http_client.h b/api/include/opentelemetry/http/http_client.h
@@ -78,7 +78,7 @@ class HttpRequest
   virtual void SetUrl(nostd::string_view const &url) = 0;
 
   // Gets the request Headers
-  virtual HttpHeaders *GetHeaders() const = 0;
+  virtual HttpHeaders &GetHeaders() const = 0;
 
   // Sets the request body
   virtual void SetBody(const uint8_t *const body, const size_t len) = 0;
@@ -106,7 +106,7 @@ class HttpResponse
   virtual unsigned GetStatusCode() = 0;
 
   // Gets the response headers.
-  virtual HttpHeaders *GetHeaders() = 0;
+  virtual const HttpHeaders &GetHeaders() = 0;
 
   // Gets the response body.
   virtual void GetBody(uint8_t *body, size_t &len) = 0;

diff --git a/sdk/include/opentelemetry/sdk/http/http_client.h b/sdk/include/opentelemetry/sdk/http/http_client.h
@@ -16,11 +16,12 @@
 #include "opentelemetry/http/http_client.h"
 #include "opentelemetry/version.h"
 
-#include <map>
-#include <vector>
-#include <cstring>
 #include <algorithm>
+#include <cstring>
 #include <functional>
+#include <map>
+#include <memory>
+#include <vector>
 
 OPENTELEMETRY_BEGIN_NAMESPACE
 namespace sdk
@@ -29,18 +30,19 @@ namespace http
 {
 namespace http_api = opentelemetry::http;
 
-static std::function <bool (const std::string&, const std::string&)> CaseInsensitiveComparator
-    = [](const std::string& s1, const std::string& s2) -> bool {
-        std::string str1(s1.length(),' ');
-        std::string str2(s2.length(),' ');
-        auto lowerCase = [](char c) -> char { return tolower(c);};
-        std::transform(s1.begin(), s1.end(), str1.begin(), lowerCase);
-        std::transform(s2.begin(), s2.end(), str2.begin(), lowerCase);
-        return  str1 < str2;
-    };
+static std::function<bool(const std::string &, const std::string &)> CaseInsensitiveComparator =
+    [](const std::string &s1, const std::string &s2) -> bool {
+  std::string str1(s1.length(), ' ');
+  std::string str2(s2.length(), ' ');
+  auto lowerCase = [](char c) -> char { return tolower(c); };
+  std::transform(s1.begin(), s1.end(), str1.begin(), lowerCase);
+  std::transform(s2.begin(), s2.end(), str2.begin(), lowerCase);
+  return str1 < str2;
+};
 
 // HttpHeaders implementation
-class HttpHeaders : http_api::HttpHeaders, std::multimap<std::string, std::string, decltype(CaseInsensitiveComparator)>
+class HttpHeaders : http_api::HttpHeaders,
+                    std::multimap<std::string, std::string, decltype(CaseInsensitiveComparator)>
 {
 public:
   virtual void set(nostd::string_view const &name, nostd::string_view const &value) override
@@ -87,7 +89,7 @@ class SimpleHttpRequest : public http_api::HttpRequest
   virtual void SetUrl(nostd::string_view const &url) override { url_ = url; }
 
   // Gets the HTTP request headers.
-  virtual http_api::HttpHeaders *GetHeaders() const override { return headers_; }
+  virtual http_api::HttpHeaders &GetHeaders() const override { return *headers_; }
 
   // Sets the request body.
   virtual void SetBody(const uint8_t *const body, const size_t len) override
@@ -109,7 +111,7 @@ class SimpleHttpRequest : public http_api::HttpRequest
   }
 
 private:
-  http_api::HttpHeaders *headers_;
+  std::unique_ptr<http_api::HttpHeaders> headers_;
   std::vector<uint8_t> body_;
   nostd::string_view method_;
   nostd::string_view id_;
@@ -128,7 +130,7 @@ class SimpleHttpResponse : public http_api::HttpResponse
 
   virtual unsigned GetStatusCode() override { return statusCode_; }
 
-  virtual http_api::HttpHeaders *GetHeaders() override { return headers_; }
+  virtual const http_api::HttpHeaders &GetHeaders() override { return *headers_; }
 
   virtual void GetBody(uint8_t *body, size_t &len) override
   {
@@ -142,7 +144,7 @@ class SimpleHttpResponse : public http_api::HttpResponse
 private:
   nostd::string_view id_;
   unsigned statusCode_;
-  http_api::HttpHeaders *headers_;
+  std::unique_ptr<http_api::HttpHeaders> headers_;
   http_api::HttpResult result_;
   std::vector<uint8_t> body_;
 };