From 559d39e57113a790d29cd26dd55e678b6594fb00 Mon Sep 17 00:00:00 2001 From: Corey Kosak Date: Wed, 26 Jun 2024 19:32:41 -0400 Subject: [PATCH] C++ Client: fix WAvg, fix noexcept, optimize PercentileBy, fix comments (#5684) --- .../deephaven/client/impl/table_handle_impl.h | 3 -- .../include/public/deephaven/client/client.h | 45 ++++++++++++------- cpp-client/deephaven/dhclient/src/client.cc | 15 +++---- .../dhclient/src/impl/table_handle_impl.cc | 5 --- 4 files changed, 34 insertions(+), 34 deletions(-) diff --git a/cpp-client/deephaven/dhclient/include/private/deephaven/client/impl/table_handle_impl.h b/cpp-client/deephaven/dhclient/include/private/deephaven/client/impl/table_handle_impl.h index 6229befb7bd..378aa77d710 100644 --- a/cpp-client/deephaven/dhclient/include/private/deephaven/client/impl/table_handle_impl.h +++ b/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 { std::vector column_specs); [[nodiscard]] std::shared_ptr - PercentileBy(double percentile, std::vector column_specs); - [[nodiscard]] - std::shared_ptr CountBy(std::string count_by_column, std::vector column_specs); [[nodiscard]] std::shared_ptr diff --git a/cpp-client/deephaven/dhclient/include/public/deephaven/client/client.h b/cpp-client/deephaven/dhclient/include/public/deephaven/client/client.h index 24c8f4924ae..8a00f38ce9d 100644 --- a/cpp-client/deephaven/dhclient/include/public/deephaven/client/client.h +++ b/cpp-client/deephaven/dhclient/include/public/deephaven/client/client.h @@ -372,7 +372,7 @@ class Aggregate { /** * Copy constructor */ - Aggregate(const Aggregate &other) noexcept; + Aggregate(const Aggregate &other); /** * Move constructor */ @@ -380,7 +380,7 @@ class Aggregate { /** * 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 + template [[nodiscard]] - static Aggregate WAvg(Args &&...args) { + static Aggregate WAvg(WeightArg &&weight_column, Args &&...args) { + auto weight = internal::ConvertToString::ToString(std::forward(weight_column)); std::vector vec{internal::ConvertToString::ToString(std::forward(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 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) 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 @@ -1107,7 +1116,7 @@ class TableHandle { * A variadic form of By(AggregateCombo, std::vector) 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 @@ -1129,7 +1138,7 @@ class TableHandle { * A variadic form of MinBy(std::vector) 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 @@ -1151,7 +1160,7 @@ class TableHandle { * A variadic form of MaxBy(std::vector) 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 @@ -1173,7 +1182,7 @@ class TableHandle { * A variadic form of SumBy(std::vector) 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 @@ -1195,7 +1204,7 @@ class TableHandle { * A variadic form of AbsSumBy(std::vector) 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 @@ -1217,7 +1226,7 @@ class TableHandle { * A variadic form of VarBy(std::vector) 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 @@ -1370,7 +1379,9 @@ class TableHandle { * @return A TableHandle referencing the new table */ [[nodiscard]] - TableHandle PercentileBy(double percentile, std::vector column_specs) const; + TableHandle PercentileBy(double percentile, std::vector column_specs) const { + return PercentileBy(percentile, false, std::move(column_specs)); + } /** * A variadic form of PercentileBy(double, std::vector) const that takes a combination of * argument types. @@ -1382,7 +1393,7 @@ class TableHandle { [[nodiscard]] TableHandle PercentileBy(double percentile, Args &&...args) const { std::vector vec{internal::ConvertToString::ToString(std::forward(args))...}; - return PercentileBy(percentile, std::move(vec)); + return PercentileBy(percentile, false, std::forward(args...)); } /** @@ -1876,7 +1887,7 @@ class TableHandle { /** * Unsubscribe from the table. */ - void Unsubscribe(std::shared_ptr callback); + void Unsubscribe(const std::shared_ptr &handle); /** * Get access to the bytes of the Deephaven "Ticket" type (without having to reference the diff --git a/cpp-client/deephaven/dhclient/src/client.cc b/cpp-client/deephaven/dhclient/src/client.cc index c6ee7a5c462..18eaf3f9e7e 100644 --- a/cpp-client/deephaven/dhclient/src/client.cc +++ b/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 vec) { } AggregateCombo::AggregateCombo(std::shared_ptr 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 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 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 callback) { - impl_->Unsubscribe(std::move(callback)); +void TableHandle::Unsubscribe(const std::shared_ptr &handle) { + impl_->Unsubscribe(handle); } const std::string &TableHandle::GetTicketAsString() const { diff --git a/cpp-client/deephaven/dhclient/src/impl/table_handle_impl.cc b/cpp-client/deephaven/dhclient/src/impl/table_handle_impl.cc index 81d8251fc0a..7e7648c201f 100644 --- a/cpp-client/deephaven/dhclient/src/impl/table_handle_impl.cc +++ b/cpp-client/deephaven/dhclient/src/impl/table_handle_impl.cc @@ -278,11 +278,6 @@ std::shared_ptr TableHandleImpl::PercentileBy(double percentile return DefaultAggregateByDescriptor(std::move(descriptor), std::move(column_specs)); } -std::shared_ptr TableHandleImpl::PercentileBy(double percentile, - std::vector column_specs) { - return PercentileBy(percentile, false, std::move(column_specs)); -} - std::shared_ptr TableHandleImpl::CountBy(std::string count_by_column, std::vector column_specs) { ComboAggregateRequest::Aggregate descriptor;