use return values instead of output parameters

dgoffredo · dgoffredo · commit 7fca52537f28 · 2022-11-02T03:10:03.000-04:00
diff --git a/src/datadog/collector.h b/src/datadog/collector.h
@@ -33,15 +33,14 @@ class Collector {
       std::vector<std::unique_ptr<SpanData>>&& spans,
       const std::shared_ptr<TraceSampler>& response_handler) = 0;
 
-  // Assign to the specified `destination` a JSON representation of this
-  // object's configuration. The JSON representation is an object with
-  // the following properties:
+  // Return a JSON representation of this object's configuration. The JSON
+  // representation is an object with the following properties:
   //
   // - "type" is the unmangled, qualified name of the most-derived class, e.g.
   //   "datadog::tracing::DatadogAgent".
   // - "config" is an object containing this object's configuration. "config"
   //   may be omitted if the derived class has no configuration.
-  virtual void config_json(nlohmann::json& destination) const = 0;
+  virtual nlohmann::json config_json() const = 0;
 
   virtual ~Collector() {}
 };
diff --git a/src/datadog/curl.cpp b/src/datadog/curl.cpp
@@ -126,8 +126,8 @@ void Curl::drain(std::chrono::steady_clock::time_point deadline) {
   impl_->drain(deadline);
 }
 
-void Curl::config_json(nlohmann::json &destination) const {
-  destination = nlohmann::json::object({{"type", "datadog::tracing::Curl"}});
+nlohmann::json Curl::config_json() const {
+  return nlohmann::json::object({{"type", "datadog::tracing::Curl"}});
 }
 
 CurlImpl::CurlImpl(const std::shared_ptr<Logger> &logger)
diff --git a/src/datadog/curl.h b/src/datadog/curl.h
@@ -37,7 +37,7 @@ class Curl : public HTTPClient {
 
   void drain(std::chrono::steady_clock::time_point deadline) override;
 
-  void config_json(nlohmann::json& destination) const override;
+  nlohmann::json config_json() const override;
 };
 
 }  // namespace tracing
diff --git a/src/datadog/datadog_agent.cpp b/src/datadog/datadog_agent.cpp
@@ -164,23 +164,20 @@ Expected<void> DatadogAgent::send(
   return std::nullopt;
 }
 
-void DatadogAgent::config_json(nlohmann::json& destination) const {
+nlohmann::json DatadogAgent::config_json() const {
   const auto& url = traces_endpoint_;  // brevity
   const auto flush_interval_milliseconds =
       std::chrono::duration_cast<std::chrono::milliseconds>(flush_interval_)
           .count();
-  nlohmann::json http_client_json, event_scheduler_json;
-  http_client_->config_json(http_client_json);
-  event_scheduler_->config_json(event_scheduler_json);
 
   // clang-format off
-  destination = nlohmann::json::object({
+  return nlohmann::json::object({
     {"type", "datadog::tracing::DatadogAgent"},
     {"config", nlohmann::json::object({
       {"url", (url.scheme + "://" + url.authority + url.path)},
       {"flush_interval_milliseconds", flush_interval_milliseconds},
-      {"http_client", http_client_json},
-      {"event_scheduler", event_scheduler_json},
+      {"http_client", http_client_->config_json()},
+      {"event_scheduler", event_scheduler_->config_json()},
     })},
   });
   // clang-format on
diff --git a/src/datadog/datadog_agent.h b/src/datadog/datadog_agent.h
@@ -55,7 +55,7 @@ class DatadogAgent : public Collector {
       std::vector<std::unique_ptr<SpanData>>&& spans,
       const std::shared_ptr<TraceSampler>& response_handler) override;
 
-  void config_json(nlohmann::json& destination) const override;
+  nlohmann::json config_json() const override;
 };
 
 }  // namespace tracing
diff --git a/src/datadog/environment.cpp b/src/datadog/environment.cpp
@@ -19,13 +19,16 @@ std::optional<std::string_view> lookup(Variable variable) {
   return std::string_view{value};
 }
 
-void to_json(nlohmann::json &destination) {
-  destination = nlohmann::json::object({});
+nlohmann::json to_json() {
+  auto result = nlohmann::json::object({});
+
   for (const char *name : variable_names) {
     if (const char *value = std::getenv(name)) {
-      destination[name] = value;
+      result[name] = value;
     }
   }
+
+  return result;
 }
 
 }  // namespace environment
diff --git a/src/datadog/environment.h b/src/datadog/environment.h
@@ -72,7 +72,7 @@ std::string_view name(Variable variable);
 // `std::nullptr` if that variable is not set in the environment.
 std::optional<std::string_view> lookup(Variable variable);
 
-void to_json(nlohmann::json& destination);
+nlohmann::json to_json();
 
 }  // namespace environment
 }  // namespace tracing
diff --git a/src/datadog/event_scheduler.h b/src/datadog/event_scheduler.h
@@ -30,15 +30,14 @@ class EventScheduler {
       std::chrono::steady_clock::duration interval,
       std::function<void()> callback) = 0;
 
-  // Assign to the specified `destination` a JSON representation of this
-  // object's configuration. The JSON representation is an object with
-  // the following properties:
+  // Return a JSON representation of this object's configuration. The JSON
+  // representation is an object with the following properties:
   //
   // - "type" is the unmangled, qualified name of the most-derived class, e.g.
   //   "datadog::tracing::ThreadedEventScheduler".
   // - "config" is an object containing this object's configuration. "config"
   //   may be omitted if the derived class has no configuration.
-  virtual void config_json(nlohmann::json& destination) const = 0;
+  virtual nlohmann::json config_json() const = 0;
 
   virtual ~EventScheduler() = default;
 };
diff --git a/src/datadog/http_client.h b/src/datadog/http_client.h
@@ -52,15 +52,14 @@ class HTTPClient {
   // `deadline`.
   virtual void drain(std::chrono::steady_clock::time_point deadline) = 0;
 
-  // Assign to the specified `destination` a JSON representation of this
-  // object's configuration. The JSON representation is an object with
-  // the following properties:
+  // Return a JSON representation of this object's configuration. The JSON
+  // representation is an object with the following properties:
   //
   // - "type" is the unmangled, qualified name of the most-derived class, e.g.
   //   "datadog::tracing::Curl".
   // - "config" is an object containing this object's configuration. "config"
   //   may be omitted if the derived class has no configuration.
-  virtual void config_json(nlohmann::json& destination) const = 0;
+  virtual nlohmann::json config_json() const = 0;
 
   virtual ~HTTPClient() = default;
 };
diff --git a/src/datadog/null_collector.cpp b/src/datadog/null_collector.cpp
@@ -5,9 +5,9 @@
 namespace datadog {
 namespace tracing {
 
-void NullCollector::config_json(nlohmann::json& destination) const {
+nlohmann::json NullCollector::config_json() const {
   // clang-format off
-    destination = nlohmann::json::object({
+    return nlohmann::json::object({
         {"type", "datadog::tracing::NullCollector"},
         {"config", nlohmann::json::object({})},
     });
diff --git a/src/datadog/null_collector.h b/src/datadog/null_collector.h
@@ -16,7 +16,7 @@ class NullCollector : public Collector {
     return {};
   }
 
-  void config_json(nlohmann::json& destination) const override;
+  nlohmann::json config_json() const override;
 };
 
 }  // namespace tracing
diff --git a/src/datadog/propagation_styles.cpp b/src/datadog/propagation_styles.cpp
@@ -8,15 +8,15 @@
 namespace datadog {
 namespace tracing {
 
-void to_json(nlohmann::json& destination, const PropagationStyles& styles) {
+nlohmann::json to_json(const PropagationStyles& styles) {
   std::vector<std::string> selected_names;
   if (styles.datadog) {
     selected_names.emplace_back("datadog");
   }
   if (styles.b3) {
     selected_names.emplace_back("B3");
   }
-  destination = selected_names;
+  return selected_names;
 }
 
 }  // namespace tracing
diff --git a/src/datadog/propagation_styles.h b/src/datadog/propagation_styles.h
@@ -15,7 +15,7 @@ struct PropagationStyles {
   bool b3 = false;
 };
 
-void to_json(nlohmann::json& destination, const PropagationStyles&);
+nlohmann::json to_json(const PropagationStyles&);
 
 }  // namespace tracing
 }  // namespace datadog
diff --git a/src/datadog/span_defaults.cpp b/src/datadog/span_defaults.cpp
@@ -12,17 +12,18 @@ bool operator==(const SpanDefaults& left, const SpanDefaults& right) {
 #undef EQ
 }
 
-void to_json(nlohmann::json& destination, const SpanDefaults& defaults) {
-  destination = nlohmann::json::object({});
+nlohmann::json to_json(const SpanDefaults& defaults) {
+  auto result = nlohmann::json::object({});
 #define TO_JSON(FIELD) \
-  if (!defaults.FIELD.empty()) destination[#FIELD] = defaults.FIELD
+  if (!defaults.FIELD.empty()) result[#FIELD] = defaults.FIELD
   TO_JSON(service);
   TO_JSON(service_type);
   TO_JSON(environment);
   TO_JSON(version);
   TO_JSON(name);
   TO_JSON(tags);
 #undef TO_JSON
+  return result;
 }
 
 }  // namespace tracing
diff --git a/src/datadog/span_defaults.h b/src/datadog/span_defaults.h
@@ -21,7 +21,7 @@ struct SpanDefaults {
   std::unordered_map<std::string, std::string> tags;
 };
 
-void to_json(nlohmann::json& destination, const SpanDefaults&);
+nlohmann::json to_json(const SpanDefaults&);
 
 bool operator==(const SpanDefaults&, const SpanDefaults&);
 
diff --git a/src/datadog/span_matcher.cpp b/src/datadog/span_matcher.cpp
@@ -19,13 +19,13 @@ bool is_match(std::string_view pattern, std::string_view subject) {
 
 }  // namespace
 
-std::string SpanMatcher::to_json() const {
-  nlohmann::json object;
-  object["service"] = service;
-  object["name"] = name;
-  object["resource"] = resource;
-  object["tags"] = tags;
-  return object.dump();
+nlohmann::json SpanMatcher::to_json() const {
+  return nlohmann::json::object({
+      {"service", service},
+      {"name", name},
+      {"resource", resource},
+      {"tags", tags},
+  });
 }
 
 bool SpanMatcher::match(const SpanData& span) const {
diff --git a/src/datadog/span_matcher.h b/src/datadog/span_matcher.h
@@ -29,7 +29,7 @@ struct SpanMatcher {
   std::unordered_map<std::string, std::string> tags;
 
   bool match(const SpanData&) const;
-  std::string to_json() const;
+  nlohmann::json to_json() const;
 
   static Expected<SpanMatcher> from_json(const nlohmann::json&);
 };
diff --git a/src/datadog/span_sampler.cpp b/src/datadog/span_sampler.cpp
@@ -66,14 +66,13 @@ SpanSampler::Rule* SpanSampler::match(const SpanData& span) {
   return nullptr;
 }
 
-void SpanSampler::config_json(nlohmann::json& destination) const {
+nlohmann::json SpanSampler::config_json() const {
   std::vector<nlohmann::json> rules;
   for (const auto& rule : rules_) {
-    rules.emplace_back();
-    to_json(rules.back(), rule);
+    rules.push_back(to_json(rule));
   }
 
-  destination = nlohmann::json::object({
+  return nlohmann::json::object({
       {"rules", rules},
   });
 }
diff --git a/src/datadog/span_sampler.h b/src/datadog/span_sampler.h
@@ -60,7 +60,7 @@ class SpanSampler {
   // return null if there is no match.
   Rule* match(const SpanData&);
 
-  void config_json(nlohmann::json& destination) const;
+  nlohmann::json config_json() const;
 };
 
 }  // namespace tracing
diff --git a/src/datadog/span_sampler_config.cpp b/src/datadog/span_sampler_config.cpp
@@ -213,7 +213,7 @@ Expected<FinalizedSpanSamplerConfig> finalize_config(
       prefix +=
           "Unable to parse sample_rate in span sampling rule with span "
           "pattern ";
-      prefix += rule.to_json();
+      prefix += rule.to_json().dump();
       prefix += ": ";
       return error->with_prefix(prefix);
     }
@@ -226,7 +226,7 @@ Expected<FinalizedSpanSamplerConfig> finalize_config(
              std::end(allowed_types))) {
       std::string message;
       message += "Span sampling rule with pattern ";
-      message += rule.to_json();
+      message += rule.to_json().dump();
       message +=
           " should have a max_per_second value greater than zero, but the "
           "following value was given: ";
@@ -244,18 +244,19 @@ Expected<FinalizedSpanSamplerConfig> finalize_config(
   return result;
 }
 
-void to_json(nlohmann::json &destination,
-             const FinalizedSpanSamplerConfig::Rule &rule) {
-  destination = nlohmann::json::object({
+nlohmann::json to_json(const FinalizedSpanSamplerConfig::Rule &rule) {
+  auto result = nlohmann::json::object({
       {"service", rule.service},
       {"name", rule.name},
       {"resource", rule.resource},
       {"sample_rate", double(rule.sample_rate)},
   });
 
   if (rule.max_per_second) {
-    destination["max_per_second"] = *rule.max_per_second;
+    result["max_per_second"] = *rule.max_per_second;
   }
+
+  return result;
 }
 
 }  // namespace tracing
diff --git a/src/datadog/span_sampler_config.h b/src/datadog/span_sampler_config.h
@@ -54,8 +54,7 @@ class FinalizedSpanSamplerConfig {
 Expected<FinalizedSpanSamplerConfig> finalize_config(const SpanSamplerConfig&,
                                                      Logger&);
 
-void to_json(nlohmann::json& destination,
-             const FinalizedSpanSamplerConfig::Rule&);
+nlohmann::json to_json(const FinalizedSpanSamplerConfig::Rule&);
 
 }  // namespace tracing
 }  // namespace datadog
diff --git a/src/datadog/threaded_event_scheduler.cpp b/src/datadog/threaded_event_scheduler.cpp
@@ -58,8 +58,8 @@ EventScheduler::Cancel ThreadedEventScheduler::schedule_recurring_event(
   };
 }
 
-void ThreadedEventScheduler::config_json(nlohmann::json& destination) const {
-  destination = nlohmann::json::object(
+nlohmann::json ThreadedEventScheduler::config_json() const {
+  return nlohmann::json::object(
       {{"type", "datadog::tracing::ThreadedEventScheduler"}});
 }
 
diff --git a/src/datadog/threaded_event_scheduler.h b/src/datadog/threaded_event_scheduler.h
@@ -57,7 +57,7 @@ class ThreadedEventScheduler : public EventScheduler {
   Cancel schedule_recurring_event(std::chrono::steady_clock::duration interval,
                                   std::function<void()> callback) override;
 
-  void config_json(nlohmann::json& destination) const override;
+  nlohmann::json config_json() const override;
 };
 
 }  // namespace tracing
diff --git a/src/datadog/trace_sampler.cpp b/src/datadog/trace_sampler.cpp
@@ -97,14 +97,13 @@ void TraceSampler::handle_collector_response(
   collector_sample_rates_ = response.sample_rate_by_key;
 }
 
-void TraceSampler::config_json(nlohmann::json& destination) const {
+nlohmann::json TraceSampler::config_json() const {
   std::vector<nlohmann::json> rules;
   for (const auto& rule : rules_) {
-    rules.emplace_back();
-    to_json(rules.back(), rule);
+    rules.push_back(to_json(rule));
   }
 
-  destination = nlohmann::json::object({
+  return nlohmann::json::object({
       {"rules", rules},
       {"max_per_second", limiter_max_per_second_},
   });
diff --git a/src/datadog/trace_sampler.h b/src/datadog/trace_sampler.h
@@ -121,7 +121,7 @@ class TraceSampler {
   // collector response.
   void handle_collector_response(const CollectorResponse&);
 
-  void config_json(nlohmann::json& destination) const;
+  nlohmann::json config_json() const;
 };
 
 }  // namespace tracing
diff --git a/src/datadog/trace_sampler_config.cpp b/src/datadog/trace_sampler_config.cpp
diff --git a/src/datadog/trace_sampler_config.h b/src/datadog/trace_sampler_config.h
diff --git a/src/datadog/tracer.cpp b/src/datadog/tracer.cpp
diff --git a/test/mocks/collectors.cpp b/test/mocks/collectors.cpp
diff --git a/test/mocks/collectors.h b/test/mocks/collectors.h
diff --git a/test/mocks/event_schedulers.h b/test/mocks/event_schedulers.h
diff --git a/test/mocks/http_clients.h b/test/mocks/http_clients.h

Original file line number	Diff line number	Diff line change
`@@ -126,8 +126,8 @@ void Curl::drain(std::chrono::steady_clock::time_point deadline) {`
`126`	`126`	`impl_->drain(deadline);`
`127`	`127`	`}`
`128`	`128`
`129`		`-void Curl::config_json(nlohmann::json &destination) const {`
`130`		`- destination = nlohmann::json::object({{"type", "datadog::tracing::Curl"}});`
	`129`	`+nlohmann::json Curl::config_json() const {`
	`130`	`+ return nlohmann::json::object({{"type", "datadog::tracing::Curl"}});`
`131`	`131`	`}`
`132`	`132`
`133`	`133`	`CurlImpl::CurlImpl(const std::shared_ptr<Logger> &logger)`
Original file line number	Diff line number	Diff line change
`@@ -19,13 +19,16 @@ std::optional<std::string_view> lookup(Variable variable) {`
`19`	`19`	`return std::string_view{value};`
`20`	`20`	`}`
`21`	`21`
`22`		`-void to_json(nlohmann::json &destination) {`
`23`		`- destination = nlohmann::json::object({});`
	`22`	`+nlohmann::json to_json() {`
	`23`	`+ auto result = nlohmann::json::object({});`
	`24`	`+`
`24`	`25`	`for (const char *name : variable_names) {`
`25`	`26`	`if (const char *value = std::getenv(name)) {`
`26`		`- destination[name] = value;`
	`27`	`+ result[name] = value;`
`27`	`28`	`}`
`28`	`29`	`}`
	`30`	`+`
	`31`	`+ return result;`
`29`	`32`	`}`
`30`	`33`
`31`	`34`	`} // namespace environment`