From edb288193f6c5fe67e113f62a7d0b130684ad827 Mon Sep 17 00:00:00 2001 From: Thomas Nixon Date: Tue, 16 Jan 2024 10:47:10 +0000 Subject: [PATCH 1/8] NamedType: add mechanism to set default value --- include/adm/detail/named_type.hpp | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/include/adm/detail/named_type.hpp b/include/adm/detail/named_type.hpp index 35ae799b..68333bb9 100644 --- a/include/adm/detail/named_type.hpp +++ b/include/adm/detail/named_type.hpp @@ -9,6 +9,15 @@ namespace adm { /// @brief Implementation details namespace detail { + /// Get the default value for some NamedType; specialise this to add custom + /// defaults. Note that this is mainly used to make NamedType values + /// default-constructable when the validator doesn't accept the default + /// value of the underlying type. + template + NT getNamedTypeDefault() { + return NT{typename NT::value_type{}}; + } + /** * @brief Named type class * @@ -22,7 +31,8 @@ namespace adm { typedef T value_type; typedef Tag tag; - NamedType() : value_() { Validator::validate(get()); } + NamedType() : NamedType(getNamedTypeDefault()) {} + explicit NamedType(T const& value) : value_(value) { Validator::validate(get()); } From 32d63048dbea2816ac0f5b914921831777a36918 Mon Sep 17 00:00:00 2001 From: Thomas Nixon Date: Tue, 16 Jan 2024 11:55:24 +0000 Subject: [PATCH 2/8] NamedType: add defaults for NamedTypes with validators --- include/adm/elements/common_parameters.hpp | 10 +++++----- include/adm/elements/frequency.hpp | 7 +++++++ .../adm/elements/gain_interaction_range.hpp | 18 ++++++++++++++++++ .../elements/position_interaction_range.hpp | 15 +++++++++++++++ include/adm/elements/position_types.hpp | 14 ++++++++++++++ include/adm/elements/screen_edge_lock.hpp | 17 +++++++++++++++++ include/adm/elements/speaker_position.hpp | 7 +++++++ 7 files changed, 83 insertions(+), 5 deletions(-) diff --git a/include/adm/elements/common_parameters.hpp b/include/adm/elements/common_parameters.hpp index ff8348f6..235253f2 100644 --- a/include/adm/elements/common_parameters.hpp +++ b/include/adm/elements/common_parameters.hpp @@ -13,7 +13,7 @@ namespace adm { namespace detail { template <> - inline Importance getDefault() { + inline Importance getNamedTypeDefault() { return Importance{10}; } @@ -23,22 +23,22 @@ namespace adm { } template <> - inline Rtime getDefault() { + inline Rtime getNamedTypeDefault() { return Rtime{std::chrono::nanoseconds{0}}; } template <> - inline HeadLocked getDefault() { + inline HeadLocked getNamedTypeDefault() { return HeadLocked{false}; } template <> - inline ScreenRef getDefault() { + inline ScreenRef getNamedTypeDefault() { return ScreenRef{false}; } template <> - inline Normalization getDefault() { + inline Normalization getNamedTypeDefault() { return Normalization{"SN3D"}; } diff --git a/include/adm/elements/frequency.hpp b/include/adm/elements/frequency.hpp index 79dd9b79..71d1b190 100644 --- a/include/adm/elements/frequency.hpp +++ b/include/adm/elements/frequency.hpp @@ -35,6 +35,13 @@ namespace adm { */ using LowPass = detail::NamedType>; + namespace detail { + template <> + inline FrequencyType getNamedTypeDefault() { + return FrequencyType{"lowPass"}; + } + } // namespace detail + /// @brief Tag for Frequency class struct FrequencyTag {}; /** diff --git a/include/adm/elements/gain_interaction_range.hpp b/include/adm/elements/gain_interaction_range.hpp index bc6cfb71..d41874a6 100644 --- a/include/adm/elements/gain_interaction_range.hpp +++ b/include/adm/elements/gain_interaction_range.hpp @@ -35,6 +35,24 @@ namespace adm { detail::NamedType; + namespace detail { + template <> + inline GainInteractionMin getNamedTypeDefault() { + return GainInteractionMin{Gain::fromLinear(1.0)}; + } + + template <> + inline GainInteractionMax getNamedTypeDefault() { + return GainInteractionMax{Gain::fromLinear(1.0)}; + } + + template <> + inline GainInteractionBoundValue + getNamedTypeDefault() { + return GainInteractionBoundValue{"min"}; + } + } // namespace detail + /// @brief Tag for GainInteractionRange class struct GainInteractionRangeTag {}; /** diff --git a/include/adm/elements/position_interaction_range.hpp b/include/adm/elements/position_interaction_range.hpp index 889da1fd..135652e1 100644 --- a/include/adm/elements/position_interaction_range.hpp +++ b/include/adm/elements/position_interaction_range.hpp @@ -91,6 +91,21 @@ namespace adm { using CoordinateInteractionValue = detail::NamedType; + + namespace detail { + template <> + inline PositionInteractionBoundValue + getNamedTypeDefault() { + return PositionInteractionBoundValue{"min"}; + } + + template <> + inline CoordinateInteractionValue + getNamedTypeDefault() { + return CoordinateInteractionValue{"azimuth"}; + } + } // namespace detail + /// @brief Tag for PositionInteractionRange class struct PositionInteractionRangeTag {}; /** diff --git a/include/adm/elements/position_types.hpp b/include/adm/elements/position_types.hpp index c4aa1829..99dd9776 100644 --- a/include/adm/elements/position_types.hpp +++ b/include/adm/elements/position_types.hpp @@ -174,4 +174,18 @@ namespace adm { using CartesianCoordinateValue = detail::NamedType; + + namespace detail { + template <> + inline SphericalCoordinateValue + getNamedTypeDefault() { + return SphericalCoordinateValue{"azimuth"}; + } + + template <> + inline CartesianCoordinateValue + getNamedTypeDefault() { + return CartesianCoordinateValue{"X"}; + } + } // namespace detail } // namespace adm diff --git a/include/adm/elements/screen_edge_lock.hpp b/include/adm/elements/screen_edge_lock.hpp index cf346eea..fa73976d 100644 --- a/include/adm/elements/screen_edge_lock.hpp +++ b/include/adm/elements/screen_edge_lock.hpp @@ -36,6 +36,23 @@ namespace adm { using VerticalEdge = detail::NamedType; + namespace detail { + template <> + inline ScreenEdge getNamedTypeDefault() { + return ScreenEdge{"left"}; + } + + template <> + inline HorizontalEdge getNamedTypeDefault() { + return HorizontalEdge{"left"}; + } + + template <> + inline VerticalEdge getNamedTypeDefault() { + return VerticalEdge{"top"}; + } + } // namespace detail + /// @brief Tag for ScreenEdgeLock class struct ScreenEdgeLockTag {}; /** diff --git a/include/adm/elements/speaker_position.hpp b/include/adm/elements/speaker_position.hpp index acf32e7c..41166ac0 100644 --- a/include/adm/elements/speaker_position.hpp +++ b/include/adm/elements/speaker_position.hpp @@ -21,6 +21,13 @@ namespace adm { using BoundValue = detail::NamedType; + namespace detail { + template <> + inline BoundValue getNamedTypeDefault() { + return BoundValue{"min"}; + } + } // namespace detail + /// @brief Tag for CartesianSpeakerPosition class struct CartesianSpeakerPositionTag {}; /** From 34296392c870b7ad3f1ee939d34939b8a756e4fd Mon Sep 17 00:00:00 2001 From: Thomas Nixon Date: Tue, 16 Jan 2024 12:00:54 +0000 Subject: [PATCH 3/8] NamedType: remove non-const reference/pointer access these allow updating the value without running the validator --- include/adm/detail/named_type.hpp | 3 --- 1 file changed, 3 deletions(-) diff --git a/include/adm/detail/named_type.hpp b/include/adm/detail/named_type.hpp index 68333bb9..accf9f02 100644 --- a/include/adm/detail/named_type.hpp +++ b/include/adm/detail/named_type.hpp @@ -39,7 +39,6 @@ namespace adm { explicit NamedType(T&& value) : value_(std::move(value)) { Validator::validate(get()); } - T& get() { return value_; } T const& get() const { return value_; } bool operator==(const NamedType& other) const { @@ -117,9 +116,7 @@ namespace adm { return tmp; } - T* operator->() { return &value_; } T const* operator->() const { return &value_; } - T& operator*() { return value_; } T const& operator*() const { return value_; } private: From 0388a628393e52792c622cbbb34dd877bf384c7c Mon Sep 17 00:00:00 2001 From: Thomas Nixon Date: Tue, 16 Jan 2024 12:04:39 +0000 Subject: [PATCH 4/8] NamedType: explicitly default move and copy constructors and assignments --- include/adm/detail/named_type.hpp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/include/adm/detail/named_type.hpp b/include/adm/detail/named_type.hpp index accf9f02..b1fc4f27 100644 --- a/include/adm/detail/named_type.hpp +++ b/include/adm/detail/named_type.hpp @@ -33,6 +33,11 @@ namespace adm { NamedType() : NamedType(getNamedTypeDefault()) {} + NamedType(const NamedType&) = default; + NamedType(NamedType&&) = default; + NamedType& operator=(const NamedType&) = default; + NamedType& operator=(NamedType&&) = default; + explicit NamedType(T const& value) : value_(value) { Validator::validate(get()); } From 4bce7c57fab2baafcb651576e9a694d0e9a209f9 Mon Sep 17 00:00:00 2001 From: Thomas Nixon Date: Tue, 16 Jan 2024 12:07:07 +0000 Subject: [PATCH 5/8] NamedType: add move and copy assignments from T --- include/adm/detail/named_type.hpp | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/include/adm/detail/named_type.hpp b/include/adm/detail/named_type.hpp index b1fc4f27..66384a69 100644 --- a/include/adm/detail/named_type.hpp +++ b/include/adm/detail/named_type.hpp @@ -44,6 +44,18 @@ namespace adm { explicit NamedType(T&& value) : value_(std::move(value)) { Validator::validate(get()); } + + NamedType& operator=(const T& value) { + Validator::validate(value); + value_ = value; + return *this; + } + NamedType& operator=(T&& value) { + Validator::validate(value); + value_ = std::move(value); + return *this; + } + T const& get() const { return value_; } bool operator==(const NamedType& other) const { From 01685d67d127dfe19a893ee8ffdbf53376bbc208 Mon Sep 17 00:00:00 2001 From: Thomas Nixon Date: Tue, 16 Jan 2024 12:12:46 +0000 Subject: [PATCH 6/8] NamedType: validate in ++ and -- the mess here is to make sure that the new value is valid without changing the old value, to make sure that the value stays valid even when an exception is thrown fixes #192 --- include/adm/detail/named_type.hpp | 10 ++++++++-- tests/named_type_tests.cpp | 10 ++++++++++ 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/include/adm/detail/named_type.hpp b/include/adm/detail/named_type.hpp index 66384a69..06cb03c0 100644 --- a/include/adm/detail/named_type.hpp +++ b/include/adm/detail/named_type.hpp @@ -112,7 +112,10 @@ namespace adm { } NamedType& operator++() { - ++value_; + T tmp = value_; + tmp++; + Validator::validate(tmp); + value_ = std::move(tmp); return *this; } @@ -123,7 +126,10 @@ namespace adm { } NamedType& operator--() { - --value_; + T tmp = value_; + tmp--; + Validator::validate(tmp); + value_ = std::move(tmp); return *this; } diff --git a/tests/named_type_tests.cpp b/tests/named_type_tests.cpp index 7a14d39d..18bb3482 100644 --- a/tests/named_type_tests.cpp +++ b/tests/named_type_tests.cpp @@ -52,6 +52,16 @@ TEST_CASE("NamedType_range_check") { NamedIntegerRange goodValue(5); REQUIRE_THROWS_AS(NamedIntegerRange(12), OutOfRangeError); REQUIRE_THROWS_AS(NamedIntegerRange(-1), OutOfRangeError); + + NamedIntegerRange lower_limit(0); + REQUIRE_THROWS_AS(lower_limit--, OutOfRangeError); + REQUIRE_THROWS_AS(--lower_limit, OutOfRangeError); + REQUIRE(lower_limit == 0); + + NamedIntegerRange upper_limit(10); + REQUIRE_THROWS_AS(upper_limit++, OutOfRangeError); + REQUIRE_THROWS_AS(++upper_limit, OutOfRangeError); + REQUIRE(upper_limit == 10); } TEST_CASE("screen_edge_validator") { From ab12b2843c3a92972cca0d1b1f1b3b19a8cd75c5 Mon Sep 17 00:00:00 2001 From: Thomas Nixon Date: Tue, 16 Jan 2024 12:18:17 +0000 Subject: [PATCH 7/8] NamedType: allow moving value out of NamedType In this case the value in the NamedType needs to be left valid, and this adds some overhead: constructing the new value is offset by the overhead saved by the move, but running the validator can be expensive. --- include/adm/detail/named_type.hpp | 8 ++++++- tests/named_type_tests.cpp | 35 +++++++++++++++++++++++++++++++ 2 files changed, 42 insertions(+), 1 deletion(-) diff --git a/include/adm/detail/named_type.hpp b/include/adm/detail/named_type.hpp index 06cb03c0..5f73d1d4 100644 --- a/include/adm/detail/named_type.hpp +++ b/include/adm/detail/named_type.hpp @@ -56,7 +56,13 @@ namespace adm { return *this; } - T const& get() const { return value_; } + T const& get() const& { return value_; } + + T get() && { + T tmp = std::move(value_); + *this = getNamedTypeDefault(); + return tmp; + } bool operator==(const NamedType& other) const { return this->get() == other.get(); diff --git a/tests/named_type_tests.cpp b/tests/named_type_tests.cpp index 18bb3482..5174f6ec 100644 --- a/tests/named_type_tests.cpp +++ b/tests/named_type_tests.cpp @@ -64,6 +64,41 @@ TEST_CASE("NamedType_range_check") { REQUIRE(upper_limit == 10); } +class MoveOnly { + public: + MoveOnly() : value(0) {} + MoveOnly(int value) : value(value) {} + + MoveOnly(MoveOnly const &) = delete; + MoveOnly &operator=(MoveOnly const &) = delete; + + MoveOnly(MoveOnly &&) = default; + MoveOnly &operator=(MoveOnly &&) = default; + + int get() const { return value; } + + private: + int value; +}; + +struct MoveOnlyTag {}; +using NamedMoveOnly = adm::detail::NamedType; + +TEST_CASE("NamedType_move") { + NamedMoveOnly value{MoveOnly{1}}; + + MoveOnly moved = std::move(value).get(); + REQUIRE(moved.get() == 1); + REQUIRE(value.get().get() == 0); + + MoveOnly moved_from_temporary = std::move(NamedMoveOnly{MoveOnly{2}}).get(); + REQUIRE(moved_from_temporary.get() == 2); + + value = MoveOnly{3}; + NamedMoveOnly moved_from_named(std::move(value)); + REQUIRE(moved_from_named.get().get() == 3); +} + TEST_CASE("screen_edge_validator") { using namespace adm; struct ScreenEdgeLockTag {}; From 663ed06c17cc5f31588078294676af984123965e Mon Sep 17 00:00:00 2001 From: Thomas Nixon Date: Tue, 16 Jan 2024 13:19:01 +0000 Subject: [PATCH 8/8] make Time default-constructable --- include/adm/elements/time.hpp | 1 + 1 file changed, 1 insertion(+) diff --git a/include/adm/elements/time.hpp b/include/adm/elements/time.hpp index 65116ecb..686f30e8 100644 --- a/include/adm/elements/time.hpp +++ b/include/adm/elements/time.hpp @@ -46,6 +46,7 @@ namespace adm { /// FractionalTime class Time { public: + Time() : time(std::chrono::nanoseconds::zero()) {} // non-explicit, as this should act like a variant template // NOLINTNEXTLINE(google-explicit-constructor)