C++ Client: fix WAvg, fix noexcept, optimize PercentileBy, fix comments

cpp-client/deephaven/dhclient/include/private/deephaven/client/arrowutil/arrow_client_table.h

@@ -1,4 +1,4 @@
-/*
+/**
  * Copyright (c) 2016-2024 Deephaven Data Labs and Patent Pending
  */
 #pragma once

cpp-client/deephaven/dhclient/include/private/deephaven/client/impl/table_handle_impl.h

@@ -94,9 +94,6 @@ class TableHandleImpl : public std::enable_shared_from_this<TableHandleImpl> {
       std::vector<std::string> column_specs);
   [[nodiscard]]
   std::shared_ptr<TableHandleImpl>
-  PercentileBy(double percentile, std::vector<std::string> column_specs);
-  [[nodiscard]]
-  std::shared_ptr<TableHandleImpl>
   CountBy(std::string count_by_column, std::vector<std::string> column_specs);
   [[nodiscard]]
   std::shared_ptr<TableHandleImpl>

cpp-client/deephaven/dhclient/include/public/deephaven/client/client.h

@@ -372,15 +372,15 @@ class Aggregate {
   /**
    * Copy constructor
    */
-  Aggregate(const Aggregate &other) noexcept;
+  Aggregate(const Aggregate &other);
   /**
    * Move constructor
    */
   Aggregate(Aggregate &&other) noexcept;
   /**
    * Copy assigment operator.
    */
-  Aggregate &operator=(const Aggregate &other) noexcept;
+  Aggregate &operator=(const Aggregate &other);
   /**
    * Move assigment operator.
    */
@@ -654,11 +654,12 @@ class Aggregate {
    * @param args The arguments to WAvg
    * @return An Aggregate object representing the aggregation
    */
-  template<typename ...Args>
+  template<typename WeightArg, typename ...Args>
   [[nodiscard]]
-  static Aggregate WAvg(Args &&...args) {
+  static Aggregate WAvg(WeightArg &&weight_column, Args &&...args) {
+    auto weight = internal::ConvertToString::ToString(std::forward<WeightArg>(weight_column));
     std::vector<std::string> vec{internal::ConvertToString::ToString(std::forward<Args>(args))...};
-    return WAvg(std::move(vec));
+    return WAvg(std::move(weight), std::move(vec));
   }
 
   /**
@@ -693,11 +694,19 @@ class AggregateCombo {
   static AggregateCombo Create(std::vector<Aggregate> vec);
 
   /**
-   * Move constructor.
+ * Copy constructor
+ */
+  AggregateCombo(const AggregateCombo &other);
+  /**
+   * Move constructor
    */
   AggregateCombo(AggregateCombo &&other) noexcept;
   /**
-   * Move assignment operator.
+   * Copy assigment operator.
+   */
+  AggregateCombo &operator=(const AggregateCombo &other);
+  /**
+   * Move assigment operator.
    */
   AggregateCombo &operator=(AggregateCombo &&other) noexcept;
 
@@ -1090,7 +1099,7 @@ class TableHandle {
    * A variadic form of By(std::vector<std::string>) const that takes a combination of
    * argument types.
    * @tparam Args Any combination of `std::string`, `std::string_view`, or `const char *`
-   * @param args The columns to UpdateView
+   * @param args Columns to group by
    * @return A TableHandle referencing the new table
    */
   template<typename ...Args>
@@ -1107,7 +1116,7 @@ class TableHandle {
    * A variadic form of By(AggregateCombo, std::vector<std::string>) const that takes a combination of
    * argument types.
    * @tparam Args Any combination of `std::string`, `std::string_view`, or `const char *`
-   * @param args The columns to UpdateView
+   * @param columnSpecs Columns to group by.
    * @return A TableHandle referencing the new table
    */
   template<typename ...Args>
@@ -1129,7 +1138,7 @@ class TableHandle {
    * A variadic form of MinBy(std::vector<std::string>) const that takes a combination of
    * argument types.
    * @tparam Args Any combination of `std::string`, `std::string_view`, or `const char *`
-   * @param args The columns to UpdateView
+   * @param columnSpecs Columns to group by.
    * @return A TableHandle referencing the new table
    */
   template<typename ...Args>
@@ -1151,7 +1160,7 @@ class TableHandle {
    * A variadic form of MaxBy(std::vector<std::string>) const that takes a combination of
    * argument types.
    * @tparam Args Any combination of `std::string`, `std::string_view`, or `const char *`
-   * @param args The columns
+   * @param args Columns to group by
    * @return A TableHandle referencing the new table
    */
   template<typename ...Args>
@@ -1173,7 +1182,7 @@ class TableHandle {
    * A variadic form of SumBy(std::vector<std::string>) const that takes a combination of
    * argument types.
    * @tparam Args Any combination of `std::string`, `std::string_view`, or `const char *`
-   * @param args The columns
+   * @param columnSpecs Columns to group by.
    * @return A TableHandle referencing the new table
    */
   template<typename ...Args>
@@ -1195,7 +1204,7 @@ class TableHandle {
    * A variadic form of AbsSumBy(std::vector<std::string>) const that takes a combination of
    * argument types.
    * @tparam Args Any combination of `std::string`, `std::string_view`, or `const char *`
-   * @param args The columns
+   * @param args Columns to group by.
    * @return A TableHandle referencing the new table
    */
   template<typename ...Args>
@@ -1217,7 +1226,7 @@ class TableHandle {
    * A variadic form of VarBy(std::vector<std::string>) const that takes a combination of
    * argument types.
    * @tparam Args Any combination of `std::string`, `std::string_view`, or `const char *`
-   * @param args The columns
+   * @param args The columns to group by
    * @return A TableHandle referencing the new table
    */
   template<typename ...Args>
@@ -1370,7 +1379,9 @@ class TableHandle {
    * @return A TableHandle referencing the new table
    */
   [[nodiscard]]
-  TableHandle PercentileBy(double percentile, std::vector<std::string> column_specs) const;
+  TableHandle PercentileBy(double percentile, std::vector<std::string> column_specs) const {
+    return PercentileBy(percentile, false, std::move(column_specs));
+  }
   /**
    * A variadic form of PercentileBy(double, std::vector<std::string>) const that takes a combination of
    * argument types.
@@ -1382,7 +1393,7 @@ class TableHandle {
   [[nodiscard]]
   TableHandle PercentileBy(double percentile, Args &&...args) const {
     std::vector<std::string> vec{internal::ConvertToString::ToString(std::forward<Args>(args))...};
-    return PercentileBy(percentile, std::move(vec));
+    return PercentileBy(percentile, false, std::forward<Args...>(args...));
   }
 
   /**
@@ -1876,7 +1887,7 @@ class TableHandle {
   /**
    * Unsubscribe from the table.
    */
-  void Unsubscribe(std::shared_ptr<SubscriptionHandle> callback);
+  void Unsubscribe(const std::shared_ptr<SubscriptionHandle> &handle);
 
   /**
    * Get access to the bytes of the Deephaven "Ticket" type (without having to reference the

cpp-client/deephaven/dhclient/src/client.cc

@@ -180,9 +180,9 @@ Aggregate createAggForMatchPairs(ComboAggregateRequest::AggType aggregate_type,
 }  // namespace
 
 Aggregate::Aggregate() = default;
-Aggregate::Aggregate(const Aggregate &other) noexcept = default;
+Aggregate::Aggregate(const Aggregate &other) = default;
 Aggregate::Aggregate(Aggregate &&other) noexcept = default;
-Aggregate &Aggregate::operator=(const Aggregate &other) noexcept = default;
+Aggregate &Aggregate::operator=(const Aggregate &other) = default;
 Aggregate &Aggregate::operator=(Aggregate &&other) noexcept = default;
 Aggregate::~Aggregate() = default;
 
@@ -283,6 +283,8 @@ AggregateCombo AggregateCombo::Create(std::vector<Aggregate> vec) {
 }
 
 AggregateCombo::AggregateCombo(std::shared_ptr<impl::AggregateComboImpl> impl) : impl_(std::move(impl)) {}
+AggregateCombo::AggregateCombo(const deephaven::client::AggregateCombo &other) = default;
+AggregateCombo &AggregateCombo::operator=(const AggregateCombo &other) = default;
 AggregateCombo::AggregateCombo(AggregateCombo &&other) noexcept = default;
 AggregateCombo &AggregateCombo::operator=(AggregateCombo &&other) noexcept = default;
 AggregateCombo::~AggregateCombo() = default;
@@ -406,11 +408,6 @@ TableHandle TableHandle::PercentileBy(double percentile, bool avg_median,
   return TableHandle(std::move(qt_impl));
 }
 
-TableHandle TableHandle::PercentileBy(double percentile, std::vector<std::string> column_specs) const {
-  auto qt_impl = impl_->PercentileBy(percentile, std::move(column_specs));
-  return TableHandle(std::move(qt_impl));
-}
-
 TableHandle TableHandle::CountBy(std::string count_by_column,
     std::vector<std::string> column_specs) const {
   auto qt_impl = impl_->CountBy(std::move(count_by_column), std::move(column_specs));
@@ -571,8 +568,8 @@ TableHandle::Subscribe(onTickCallback_t on_tick, void *on_tick_user_data,
   return impl_->Subscribe(on_tick, on_tick_user_data, on_error, on_error_user_data);
 }
 
-void TableHandle::Unsubscribe(std::shared_ptr<SubscriptionHandle> callback) {
-  impl_->Unsubscribe(std::move(callback));
+void TableHandle::Unsubscribe(const std::shared_ptr<SubscriptionHandle> &handle) {
+  impl_->Unsubscribe(handle);
 }
 
 const std::string &TableHandle::GetTicketAsString() const {

cpp-client/deephaven/dhclient/src/impl/table_handle_impl.cc

@@ -278,11 +278,6 @@ std::shared_ptr<TableHandleImpl> TableHandleImpl::PercentileBy(double percentile
   return DefaultAggregateByDescriptor(std::move(descriptor), std::move(column_specs));
 }
 
-std::shared_ptr<TableHandleImpl> TableHandleImpl::PercentileBy(double percentile,
-    std::vector<std::string> column_specs) {
-  return PercentileBy(percentile, false, std::move(column_specs));
-}
-
 std::shared_ptr<TableHandleImpl> TableHandleImpl::CountBy(std::string count_by_column,
     std::vector<std::string> column_specs) {
   ComboAggregateRequest::Aggregate descriptor;